GSoC 2012 NS-3 LTE MAC scheduler project: FDMT, TDMT, TTA, FDBET, TDBET, FDTBFQ, TDTBFQ, PSS

GSoC 2012 NS-3 LTE MAC scheduler project: FDMT, TDMT, TTA, FDBET, TDBET, FDTBFQ, TDTBFQ, PSS

[Marco Miozzo, 2012-08-24 12:24:57]

You may find my first round of comments.
I have a general remark on the codestyle, which has to be improved according to
ns3 guidelines.
However, the code implemented seems to me very interesting and valuable for the
LTE module, which in fact lacks of QoS aware schedulers.

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-design.rst
File src/lte/doc/source/lte-design.rst (right):

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-design.rs...
src/lte/doc/source/lte-design.rst:661: by subband cqi. In TDMT, every TTI, MAC
scheduler selects one UE which has highest achievable rate
CQI, check also in the following text

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-design.rs...
src/lte/doc/source/lte-design.rst:686: When there are several UEs having the
same achievable rate, current implementation always select
selects

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-design.rs...
src/lte/doc/source/lte-design.rst:760: In the implementation,
:math:`r_{i}`,:math:`p_{i}`,:math:`d_{i}`,:math:`c_{i}`,:math:`C` need to be
configured in the script
This is usual part of the user section of the manual, it would suggest to
mention the parameters and their role here and move the specific command for
configuration in the proper section.

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-design.rs...
src/lte/doc/source/lte-design.rst:843: In the implementation, TBR equals to
Guarantee Bit Rate (GBR) in bearer
As for TBFQ, the actual configuration of the parameters is usually treated in
user section.

http://codereview.appspot.com/6484049/diff/1/src/lte/model/fdbet-ff-mac-sched...
File src/lte/model/fdbet-ff-mac-scheduler.cc (right):

http://codereview.appspot.com/6484049/diff/1/src/lte/model/fdbet-ff-mac-sched...
src/lte/model/fdbet-ff-mac-scheduler.cc:519: itRbgPerRntiLog =
rbgPerRntiLog.find((*itMax).first);
Check indentation.

http://codereview.appspot.com/6484049/diff/1/src/lte/model/fdbet-ff-mac-sched...
File src/lte/model/fdbet-ff-mac-scheduler.h (right):

http://codereview.appspot.com/6484049/diff/1/src/lte/model/fdbet-ff-mac-sched...
src/lte/model/fdbet-ff-mac-scheduler.h:19: *         Marco Miozzo
<marco.miozzo@cttc.es>
I would explain a bit more what is your (and my) effort on this file (I guess in
my case it's just the template you used, the PF scheduler). This consideration
is valid also for the other files.
Moreover, correct your email address.

http://codereview.appspot.com/6484049/diff/1/src/lte/model/fdtbfq-ff-mac-sche...
File src/lte/model/fdtbfq-ff-mac-scheduler.cc (right):

http://codereview.appspot.com/6484049/diff/1/src/lte/model/fdtbfq-ff-mac-sche...
src/lte/model/fdtbfq-ff-mac-scheduler.cc:557: budget = (*itMax).second.counter -
(*itMax).second.debtLimit;
Check codestyle (parenthesis and indentation)

http://codereview.appspot.com/6484049/diff/1/src/lte/model/fdtbfq-ff-mac-sche...
src/lte/model/fdtbfq-ff-mac-scheduler.cc:565: // calcualte how much bytes this
UE acutally need
actually needs

http://codereview.appspot.com/6484049/diff/1/src/lte/model/pss-ff-mac-schedul...
File src/lte/model/pss-ff-mac-scheduler.cc (right):

http://codereview.appspot.com/6484049/diff/1/src/lte/model/pss-ff-mac-schedul...
src/lte/model/pss-ff-mac-scheduler.cc:558: nMux = (int)((ueSet1.size() +
ueSet2.size()) / 2) ; // TD scheduler only transfers half selected UE per RTT to
TD scheduler
Why half, please detail since neither in the design this details is specified.

[Dizhi.Zhou, 2012-08-27 10:36:39]

Hi Marco,

Here are my reply for your comments. 

Regards
Dizhi

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-design.rst
File src/lte/doc/source/lte-design.rst (right):

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-design.rs...
src/lte/doc/source/lte-design.rst:661: by subband cqi. In TDMT, every TTI, MAC
scheduler selects one UE which has highest achievable rate
On 2012/08/24 12:24:57, marco.miozzo wrote:
> CQI, check also in the following text

Done.

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-design.rs...
src/lte/doc/source/lte-design.rst:686: When there are several UEs having the
same achievable rate, current implementation always select
On 2012/08/24 12:24:57, marco.miozzo wrote:
> selects

Done.

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-design.rs...
src/lte/doc/source/lte-design.rst:760: In the implementation,
:math:`r_{i}`,:math:`p_{i}`,:math:`d_{i}`,:math:`c_{i}`,:math:`C` need to be
configured in the script
I add a section in lte-user.rst called "Configure LTE MAC Scheduler"

On 2012/08/24 12:24:57, marco.miozzo wrote:
> This is usual part of the user section of the manual, it would suggest to
> mention the parameters and their role here and move the specific command for
> configuration in the proper section.

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-design.rs...
src/lte/doc/source/lte-design.rst:843: In the implementation, TBR equals to
Guarantee Bit Rate (GBR) in bearer
On 2012/08/24 12:24:57, marco.miozzo wrote:
> As for TBFQ, the actual configuration of the parameters is usually treated in
> user section.

Done.

http://codereview.appspot.com/6484049/diff/1/src/lte/model/fdbet-ff-mac-sched...
File src/lte/model/fdbet-ff-mac-scheduler.cc (right):

http://codereview.appspot.com/6484049/diff/1/src/lte/model/fdbet-ff-mac-sched...
src/lte/model/fdbet-ff-mac-scheduler.cc:519: itRbgPerRntiLog =
rbgPerRntiLog.find((*itMax).first);
This problem is strange. those indentions are good in my own PC. But when I
upload codes to codreview or code.nsnam.org, lots indentions are changed, I
guess it is because I use lots of vim plug-in in my PC. Now I updated codes in
my repository

On 2012/08/24 12:24:57, marco.miozzo wrote:
> Check indentation.

http://codereview.appspot.com/6484049/diff/1/src/lte/model/fdbet-ff-mac-sched...
File src/lte/model/fdbet-ff-mac-scheduler.h (right):

http://codereview.appspot.com/6484049/diff/1/src/lte/model/fdbet-ff-mac-sched...
src/lte/model/fdbet-ff-mac-scheduler.h:19: *         Marco Miozzo
<marco.miozzo@cttc.es>
Yes. I didn't consider too much on this issue. What I think initially is that
the first author is the correspondent author. so I put my name in the first...I
will reorder it by considering the contributions:) Sorry about this. 
On 2012/08/24 12:24:57, marco.miozzo wrote:
> I would explain a bit more what is your (and my) effort on this file (I guess
in
> my case it's just the template you used, the PF scheduler). This consideration
> is valid also for the other files.
> Moreover, correct your email address.

http://codereview.appspot.com/6484049/diff/1/src/lte/model/fdtbfq-ff-mac-sche...
File src/lte/model/fdtbfq-ff-mac-scheduler.cc (right):

http://codereview.appspot.com/6484049/diff/1/src/lte/model/fdtbfq-ff-mac-sche...
src/lte/model/fdtbfq-ff-mac-scheduler.cc:557: budget = (*itMax).second.counter -
(*itMax).second.debtLimit;
On 2012/08/24 12:24:57, marco.miozzo wrote:
> Check codestyle (parenthesis and indentation)

Done.

http://codereview.appspot.com/6484049/diff/1/src/lte/model/fdtbfq-ff-mac-sche...
src/lte/model/fdtbfq-ff-mac-scheduler.cc:565: // calcualte how much bytes this
UE acutally need
On 2012/08/24 12:24:57, marco.miozzo wrote:
> actually needs

Done.

http://codereview.appspot.com/6484049/diff/1/src/lte/model/pss-ff-mac-schedul...
File src/lte/model/pss-ff-mac-scheduler.cc (right):

http://codereview.appspot.com/6484049/diff/1/src/lte/model/pss-ff-mac-schedul...
src/lte/model/pss-ff-mac-scheduler.cc:558: nMux = (int)((ueSet1.size() +
ueSet2.size()) / 2) ; // TD scheduler only transfers half selected UE per RTT to
TD scheduler
There are some explanations about this question in design document (now move to
the lte-user.rst):

"In PSS, if user does not define nMux, PSS will set this value to half of total
UE."
On 2012/08/24 12:24:57, marco.miozzo wrote:
> Why half, please detail since neither in the design this details is specified.

[Nicola Baldo, 2012-08-28 10:15:42]

Overall, good work! Please find below some detailed comments. I focused mostly
on documentation and header files for now.

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-design.rst
File src/lte/doc/source/lte-design.rst (right):

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-design.rs...
src/lte/doc/source/lte-design.rst:702: Here,:math:`R_{i}(k,t)` in bit/s
represents the achievable rate for user :math:`i`
this inline math does not render properly, maybe you need to insert a space
between the "," and ":math:"

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-design.rs...
src/lte/doc/source/lte-design.rst:747: flow i needs to withdraw tokens from
token bank based on a priority metric: :math:`frac{E_{i}}{r_{i}}`, and
:math:`E_{i}` is decreased.
\frac

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-design.rs...
src/lte/doc/source/lte-design.rst:748: Obviously, the user contributes more on
token bank has higher priority to borrow tokens; on the other hand, the
maybe "the user /who/ contributes"?

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-design.rs...
src/lte/doc/source/lte-design.rst:749: user borrows more tokens from bank has
lower priority to continue to withdraw tokens. Therefore, in case of several
maybe "the user /who/ borrows"?

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-design.rs...
src/lte/doc/source/lte-design.rst:754: * Debt limit :math:`d_{i}`: if
:math:`E_{i}` belows this threshold, user i cannot further borrow tokens from
bank. This is for
:math:`i`

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-design.rs...
src/lte/doc/source/lte-design.rst:769: :math:`r_{i}` should be configured in
script and equals to the Maximum Bit Rate (MBR)
Can you explain here why MBR instead of GBR?

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-design.rs...
src/lte/doc/source/lte-design.rst:770: in bearer level QoS parameters. For
constant bit rate (CBR) traffic, it is suggested to set MBR to the traffic
generation
I would rephrase like this:

 :math:`r_{i}` is obtained from the Maximum Bit Rate (MBR) setting in bearer
level QoS parameters, which therefore needs to be configured consistently.

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-design.rs...
src/lte/doc/source/lte-design.rst:771: rate. For variance bit rate (VBR)
traffic, it is suggested to set MBR three times larger than traffic generation
rate.
You should specify here if the TBFQ scheduler additionally accounts for RLC and
PDCP overhead or not. As for the VBR suggestion, do you have any reference? Or
at least some justification? It seems a bit arbitrary... besides, I am not sure
whether "traffic generation rate" is well defined for VBR.

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-design.rs...
src/lte/doc/source/lte-design.rst:771: rate. For variance bit rate (VBR)
traffic, it is suggested to set MBR three times larger than traffic generation
rate.
For completeness, could you also specify how :math:`t_{i}` is determined in the
code?

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-design.rs...
src/lte/doc/source/lte-design.rst:777: there are no packets within UE's RLC
buffer or all RBGs are allocated[FABokhari2009]_. In TD-TBFQ, after selecting
space missing between text and citation

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-design.rs...
src/lte/doc/source/lte-design.rst:778: UE with maximum metric, it allocates all
RBGs to this UE by using wideband cqi[WKWong2004]_.
space missing between text and citation

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-design.rs...
src/lte/doc/source/lte-design.rst:784: packet scheduling operations into one
scheduler[GMonghal2008]_. It controls the fairness among UEs by a specified
space missing between text and citation

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-design.rs...
src/lte/doc/source/lte-design.rst:826: \left( \frac{ CoI[j,k] }{ CoI[j,k] }
\right)
the numberator and denominator of the fraction are equal...

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-design.rs...
src/lte/doc/source/lte-design.rst:829: difference that it is updated only when
the i-th user is actually served. :math: `CoI[j,k]` is an
this inline math does not render correctly

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-design.rs...
src/lte/doc/source/lte-design.rst:830: estimation of the SINR on the RBG :math:
`k` of UE :math: `j`. Both PFsch and CoIta is for decoupling
neither does this

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-design.rs...
src/lte/doc/source/lte-design.rst:830: estimation of the SINR on the RBG :math:
`k` of UE :math: `j`. Both PFsch and CoIta is for decoupling
this inline math does not render correctly, I guess due to whitespace after
":math:"

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-design.rs...
src/lte/doc/source/lte-design.rst:830: estimation of the SINR on the RBG :math:
`k` of UE :math: `j`. Both PFsch and CoIta is for decoupling
It's a bit misleading that you cite CoI and then use SINR. To make it clearer I
suggest that you specify which measurement provided by the FF API you are
actually using.

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-design.rs...
src/lte/doc/source/lte-design.rst:840: metric (:math:`Msch`, :math:`MCoI`) by
weight :math: `W[n]`. This strategy will guarantee the throughput of lower
inline math does not render correctly

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-testing.rst
File src/lte/doc/source/lte-testing.rst (right):

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-testing.r...
src/lte/doc/source/lte-testing.rst:425: :math: `T_i` by :math: `T`.  Then we
have:
inline maths do not render properly

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-testing.r...
src/lte/doc/source/lte-testing.rst:483: When the totol traffic rate is bigger
than :math:`T`, the UE throughput equals to :math:`\frac{T}{N}` . Otherwise, UE
throughput
total

http://codereview.appspot.com/6484049/diff/1/src/lte/model/fdbet-ff-mac-sched...
File src/lte/model/fdbet-ff-mac-scheduler.h (right):

http://codereview.appspot.com/6484049/diff/1/src/lte/model/fdbet-ff-mac-sched...
src/lte/model/fdbet-ff-mac-scheduler.h:41: 
doxygen missing

http://codereview.appspot.com/6484049/diff/1/src/lte/model/fdbet-ff-mac-sched...
src/lte/model/fdbet-ff-mac-scheduler.h:56: * \ingroup FdBetFfMacScheduler
I understand that you copied this from pf-ff-mac-scheduler.h, but this is not
correct. 
Please fix this according to bug 1499:
https://www.nsnam.org/bugzilla/show_bug.cgi?id=1499
i.e., use only "\ingroup lte" and no other \ingroup or \defgroup command

http://codereview.appspot.com/6484049/diff/1/src/lte/model/fdmt-ff-mac-schedu...
File src/lte/model/fdmt-ff-mac-scheduler.h (right):

http://codereview.appspot.com/6484049/diff/1/src/lte/model/fdmt-ff-mac-schedu...
src/lte/model/fdmt-ff-mac-scheduler.h:39: #define NO_SINR -5000
can you move this to the .cc file?

http://codereview.appspot.com/6484049/diff/1/src/lte/model/fdmt-ff-mac-schedu...
src/lte/model/fdmt-ff-mac-scheduler.h:49: * \ingroup FdMtFfMacScheduler
bug 1499 issue, see comment in fdbet-ff-mac-scheduler.h

http://codereview.appspot.com/6484049/diff/1/src/lte/model/fdtbfq-ff-mac-sche...
File src/lte/model/fdtbfq-ff-mac-scheduler.h (right):

http://codereview.appspot.com/6484049/diff/1/src/lte/model/fdtbfq-ff-mac-sche...
src/lte/model/fdtbfq-ff-mac-scheduler.h:43: 
doxygen?

http://codereview.appspot.com/6484049/diff/1/src/lte/model/fdtbfq-ff-mac-sche...
src/lte/model/fdtbfq-ff-mac-scheduler.h:47: uint64_t packetArrivalRate;     //
packet arrival rate( byte/s)
use ///< so this becomes a doxygen comment. Do the same for all member
variables.

http://codereview.appspot.com/6484049/diff/1/src/lte/model/fdtbfq-ff-mac-sche...
src/lte/model/fdtbfq-ff-mac-scheduler.h:63: * \ingroup FdTbfqFfMacScheduler
bug 1499 issue, see comment in fdbet-ff-mac-scheduler.h

http://codereview.appspot.com/6484049/diff/1/src/lte/model/fdtbfq-ff-mac-sche...
src/lte/model/fdtbfq-ff-mac-scheduler.h:68: 
remove empty line

http://codereview.appspot.com/6484049/diff/1/src/lte/model/pss-ff-mac-schedul...
File src/lte/model/pss-ff-mac-scheduler.h (right):

http://codereview.appspot.com/6484049/diff/1/src/lte/model/pss-ff-mac-schedul...
src/lte/model/pss-ff-mac-scheduler.h:43: 
doxygen

http://codereview.appspot.com/6484049/diff/1/src/lte/model/pss-ff-mac-schedul...
src/lte/model/pss-ff-mac-scheduler.h:59: * \ingroup PssFfMacScheduler
bug 1499 issue, see comment in fdbet-ff-mac-scheduler.h

http://codereview.appspot.com/6484049/diff/1/src/lte/model/tdbet-ff-mac-sched...
File src/lte/model/tdbet-ff-mac-scheduler.h (right):

http://codereview.appspot.com/6484049/diff/1/src/lte/model/tdbet-ff-mac-sched...
src/lte/model/tdbet-ff-mac-scheduler.h:42: 
doxygen

http://codereview.appspot.com/6484049/diff/1/src/lte/model/tdbet-ff-mac-sched...
src/lte/model/tdbet-ff-mac-scheduler.h:57: * \ingroup TdBetFfMacScheduler
bug 1499 issue, see comment in fdbet-ff-mac-scheduler.h

http://codereview.appspot.com/6484049/diff/1/src/lte/model/tdmt-ff-mac-schedu...
File src/lte/model/tdmt-ff-mac-scheduler.h (right):

http://codereview.appspot.com/6484049/diff/1/src/lte/model/tdmt-ff-mac-schedu...
src/lte/model/tdmt-ff-mac-scheduler.h:49: * \ingroup TdMtFfMacScheduler
bug 1499 issue, see comment in fdbet-ff-mac-scheduler.h

http://codereview.appspot.com/6484049/diff/1/src/lte/model/tdtbfq-ff-mac-sche...
File src/lte/model/tdtbfq-ff-mac-scheduler.h (right):

http://codereview.appspot.com/6484049/diff/1/src/lte/model/tdtbfq-ff-mac-sche...
src/lte/model/tdtbfq-ff-mac-scheduler.h:43: 
doxygen

http://codereview.appspot.com/6484049/diff/1/src/lte/model/tdtbfq-ff-mac-sche...
src/lte/model/tdtbfq-ff-mac-scheduler.h:63: * \ingroup TdTbfqFfMacScheduler
bug 1499 issue, see comment in fdbet-ff-mac-scheduler.h

http://codereview.appspot.com/6484049/diff/1/src/lte/model/tta-ff-mac-schedul...
File src/lte/model/tta-ff-mac-scheduler.h (right):

http://codereview.appspot.com/6484049/diff/1/src/lte/model/tta-ff-mac-schedul...
src/lte/model/tta-ff-mac-scheduler.h:49: * \ingroup TtaFfMacScheduler
bug 1499 issue, see comment in fdbet-ff-mac-scheduler.h

[Biljana Bojović, 2012-08-29 17:09:39]

I revised again documentation and here are few more comments for now.

Best regards,
Biljana Bojovic

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-design.rst
File src/lte/doc/source/lte-design.rst (right):

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-design.rs...
src/lte/doc/source/lte-design.rst:688: fairness to UEs in poor channel
condition.
Maybe it could be good to specify that is referred to  MAC layer cell
throughput, because this is not always the truth for application layer
throughput. If, for example, traffic of UE is VoIP, then UE will use just a
negligible part of assigned resources. Which means that application layer
throughput will not be always maximized by using this scheduler, rather
maximization will depend on the type of traffic.

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-design.rs...
src/lte/doc/source/lte-design.rst:730: that, in every TTI, the scheduler tries
the best to achieve the equal throughput among all UEs.
Again, depends to which throughput you are referring to.

[Dizhi.Zhou, 2012-08-31 02:42:19]

Hi Nicola and Biljana,

Thanks very much for your valuable comments. Please find my reply to those
comments below.

Best regards
Dizhi

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-design.rst
File src/lte/doc/source/lte-design.rst (right):

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-design.rs...
src/lte/doc/source/lte-design.rst:688: fairness to UEs in poor channel
condition.
I delete the first sentence of this paragraph in latest version because it will
mislead users that MT will intentionally select the first UE defined in script. 

Because the current testsuites do not use fading channel so that all UE placing
the same distance to eNB have the same CQI value. Therefore, the priority of
each UE in this case is equal with each other because all UE have the same
CQI.More complex testsuites need to be defined to reflect the unique feature of
MT scheduler.

For comment about throughput type, I think it is a good idea to add this
explanation in the beginning of scheduler testing section. I will discuss this
with Nicola and Marco later.

Actually, we always calculate throughput in RLC layer which will almost equal to
the application layer throughput (without considering the header length).
However, because current QoS unaware schedulers in NS3, such as MT, BET, TTA, PF
and RR, use RLC/SM to generates traffic, the rlc buffer is always saturated.
Thus, RLC throughput in testsuites will always equal to MAC throughput.

On 2012/08/29 17:09:39, Biljana Bojović wrote:
> Maybe it could be good to specify that is referred to  MAC layer cell
> throughput, because this is not always the truth for application layer
> throughput. If, for example, traffic of UE is VoIP, then UE will use just a
> negligible part of assigned resources. Which means that application layer
> throughput will not be always maximized by using this scheduler, rather
> maximization will depend on the type of traffic.

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-design.rs...
src/lte/doc/source/lte-design.rst:702: Here,:math:`R_{i}(k,t)` in bit/s
represents the achievable rate for user :math:`i`
On 2012/08/28 10:15:42, Nicola Baldo wrote:
> this inline math does not render properly, maybe you need to insert a space
> between the "," and ":math:"

Done.

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-design.rs...
src/lte/doc/source/lte-design.rst:730: that, in every TTI, the scheduler tries
the best to achieve the equal throughput among all UEs.
On 2012/08/29 17:09:39, Biljana Bojović wrote:
> Again, depends to which throughput you are referring to.

Done.

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-design.rs...
src/lte/doc/source/lte-design.rst:747: flow i needs to withdraw tokens from
token bank based on a priority metric: :math:`frac{E_{i}}{r_{i}}`, and
:math:`E_{i}` is decreased.
On 2012/08/28 10:15:42, Nicola Baldo wrote:
> \frac

Done.

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-design.rs...
src/lte/doc/source/lte-design.rst:748: Obviously, the user contributes more on
token bank has higher priority to borrow tokens; on the other hand, the
On 2012/08/28 10:15:42, Nicola Baldo wrote:
> maybe "the user /who/ contributes"?

Done.

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-design.rs...
src/lte/doc/source/lte-design.rst:749: user borrows more tokens from bank has
lower priority to continue to withdraw tokens. Therefore, in case of several
On 2012/08/28 10:15:42, Nicola Baldo wrote:
> maybe "the user /who/ borrows"?

Done.

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-design.rs...
src/lte/doc/source/lte-design.rst:754: * Debt limit :math:`d_{i}`: if
:math:`E_{i}` belows this threshold, user i cannot further borrow tokens from
bank. This is for
On 2012/08/28 10:15:42, Nicola Baldo wrote:
> :math:`i`

Done.

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-design.rs...
src/lte/doc/source/lte-design.rst:769: :math:`r_{i}` should be configured in
script and equals to the Maximum Bit Rate (MBR)
This is because token generation rate must equal or bigger than traffic incoming
rate. In CBR, MBR is equal to GBR. In VBR, if we set token generation rate to
GBR, then it cannot guarantee to cover the peak traffic rate. So or uniform, I
use MBR here. 


On 2012/08/28 10:15:42, Nicola Baldo wrote:
> Can you explain here why MBR instead of GBR?

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-design.rs...
src/lte/doc/source/lte-design.rst:770: in bearer level QoS parameters. For
constant bit rate (CBR) traffic, it is suggested to set MBR to the traffic
generation
On 2012/08/28 10:15:42, Nicola Baldo wrote:
> I would rephrase like this:
> 
>  :math:`r_{i}` is obtained from the Maximum Bit Rate (MBR) setting in bearer
> level QoS parameters, which therefore needs to be configured consistently.

Done.

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-design.rs...
src/lte/doc/source/lte-design.rst:771: rate. For variance bit rate (VBR)
traffic, it is suggested to set MBR three times larger than traffic generation
rate.
I rewrite this paragraph: 
In PSS, TBR is obtained from GBR in bearer level QoS parameters. In TBFQ, token
generation rate is obtained from the MBR setting in bearer level QoS parameters,
which therefore needs to be configured consistently. For constant bit rate (CBR)
traffic, it is suggested to set MBR to GBR. For variance bit rate (VBR) traffic,
it is suggested to set MBR k times larger than GBR in order to cover the peak
traffic rate. In current implementation, k is set to
three based on paper [FABokhari2009]_. In addition, current version of TBFQ does
not consider RLC header and PDCP header length in MBR and GBR. Another parameter
in TBFQ is packet arrival rate. This parameter is calculated within scheduler
and equals to the past average throughput which is used in PF scheduler.
On 2012/08/28 10:15:42, Nicola Baldo wrote:
> For completeness, could you also specify how :math:`t_{i}` is determined in
the
> code?

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-design.rs...
src/lte/doc/source/lte-design.rst:777: there are no packets within UE's RLC
buffer or all RBGs are allocated[FABokhari2009]_. In TD-TBFQ, after selecting
On 2012/08/28 10:15:42, Nicola Baldo wrote:
> space missing between text and citation

Done.

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-design.rs...
src/lte/doc/source/lte-design.rst:778: UE with maximum metric, it allocates all
RBGs to this UE by using wideband cqi[WKWong2004]_.
On 2012/08/28 10:15:42, Nicola Baldo wrote:
> space missing between text and citation

Done.

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-design.rs...
src/lte/doc/source/lte-design.rst:784: packet scheduling operations into one
scheduler[GMonghal2008]_. It controls the fairness among UEs by a specified
On 2012/08/28 10:15:42, Nicola Baldo wrote:
> space missing between text and citation

Done.

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-design.rs...
src/lte/doc/source/lte-design.rst:826: \left( \frac{ CoI[j,k] }{ CoI[j,k] }
\right)
On 2012/08/28 10:15:42, Nicola Baldo wrote:
> the numberator and denominator of the fraction are equal...

Done.

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-design.rs...
src/lte/doc/source/lte-design.rst:829: difference that it is updated only when
the i-th user is actually served. :math: `CoI[j,k]` is an
On 2012/08/28 10:15:42, Nicola Baldo wrote:
> this inline math does not render correctly

Done.

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-design.rs...
src/lte/doc/source/lte-design.rst:830: estimation of the SINR on the RBG :math:
`k` of UE :math: `j`. Both PFsch and CoIta is for decoupling
I add one sentence here: In LTE, CQI report is used as an SINR estimation

On 2012/08/28 10:15:42, Nicola Baldo wrote:
> It's a bit misleading that you cite CoI and then use SINR. To make it clearer
I
> suggest that you specify which measurement provided by the FF API you are
> actually using.

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-design.rs...
src/lte/doc/source/lte-design.rst:830: estimation of the SINR on the RBG :math:
`k` of UE :math: `j`. Both PFsch and CoIta is for decoupling
I think here should be "both". Because in the initial version of PSS, PF is used
in FD scheduler. However, the achievable rate in PF is overlapped with that in
PSS TD scheduler and thus PFsch and CoIta is used in the PSS extension.

On 2012/08/28 10:15:42, Nicola Baldo wrote:
> neither does this

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-design.rs...
src/lte/doc/source/lte-design.rst:840: metric (:math:`Msch`, :math:`MCoI`) by
weight :math: `W[n]`. This strategy will guarantee the throughput of lower
On 2012/08/28 10:15:42, Nicola Baldo wrote:
> inline math does not render correctly

Done.

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-testing.rst
File src/lte/doc/source/lte-testing.rst (right):

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-testing.r...
src/lte/doc/source/lte-testing.rst:425: :math: `T_i` by :math: `T`.  Then we
have:
On 2012/08/28 10:15:42, Nicola Baldo wrote:
> inline maths do not render properly

Done.

http://codereview.appspot.com/6484049/diff/1/src/lte/doc/source/lte-testing.r...
src/lte/doc/source/lte-testing.rst:483: When the totol traffic rate is bigger
than :math:`T`, the UE throughput equals to :math:`\frac{T}{N}` . Otherwise, UE
throughput
I think here should be UE throughput. The total throughput is T. and T/N is the
throughput of each UE.
On 2012/08/28 10:15:42, Nicola Baldo wrote:
> total

http://codereview.appspot.com/6484049/diff/1/src/lte/model/fdbet-ff-mac-sched...
File src/lte/model/fdbet-ff-mac-scheduler.h (right):

http://codereview.appspot.com/6484049/diff/1/src/lte/model/fdbet-ff-mac-sched...
src/lte/model/fdbet-ff-mac-scheduler.h:41: 
On 2012/08/28 10:15:42, Nicola Baldo wrote:
> doxygen missing

Done.

http://codereview.appspot.com/6484049/diff/1/src/lte/model/fdbet-ff-mac-sched...
src/lte/model/fdbet-ff-mac-scheduler.h:56: * \ingroup FdBetFfMacScheduler
On 2012/08/28 10:15:42, Nicola Baldo wrote:
> I understand that you copied this from pf-ff-mac-scheduler.h, but this is not
> correct. 
> Please fix this according to bug 1499:
> https://www.nsnam.org/bugzilla/show_bug.cgi?id=1499
> i.e., use only "\ingroup lte" and no other \ingroup or \defgroup command

Done.

http://codereview.appspot.com/6484049/diff/1/src/lte/model/fdmt-ff-mac-schedu...
File src/lte/model/fdmt-ff-mac-scheduler.h (right):

http://codereview.appspot.com/6484049/diff/1/src/lte/model/fdmt-ff-mac-schedu...
src/lte/model/fdmt-ff-mac-scheduler.h:39: #define NO_SINR -5000
Yes, it is better to put all the implementation code into .cc file. But I see
the RR scheduler header file also includes this line. So I move this line to .cc
file for RR scheduler. 

Also, I change the FF-API here to lte because we use the new group name here.

On 2012/08/28 10:15:42, Nicola Baldo wrote:
> can you move this to the .cc file?

http://codereview.appspot.com/6484049/diff/1/src/lte/model/fdmt-ff-mac-schedu...
src/lte/model/fdmt-ff-mac-scheduler.h:49: * \ingroup FdMtFfMacScheduler
On 2012/08/28 10:15:42, Nicola Baldo wrote:
> bug 1499 issue, see comment in fdbet-ff-mac-scheduler.h

Done.

http://codereview.appspot.com/6484049/diff/1/src/lte/model/fdtbfq-ff-mac-sche...
File src/lte/model/fdtbfq-ff-mac-scheduler.h (right):

http://codereview.appspot.com/6484049/diff/1/src/lte/model/fdtbfq-ff-mac-sche...
src/lte/model/fdtbfq-ff-mac-scheduler.h:43: 
On 2012/08/28 10:15:42, Nicola Baldo wrote:
> doxygen?

Done. I also add doxygen for PF scheduler

http://codereview.appspot.com/6484049/diff/1/src/lte/model/fdtbfq-ff-mac-sche...
src/lte/model/fdtbfq-ff-mac-scheduler.h:47: uint64_t packetArrivalRate;     //
packet arrival rate( byte/s)
On 2012/08/28 10:15:42, Nicola Baldo wrote:
> use ///< so this becomes a doxygen comment. Do the same for all member
> variables.

Done.

http://codereview.appspot.com/6484049/diff/1/src/lte/model/fdtbfq-ff-mac-sche...
src/lte/model/fdtbfq-ff-mac-scheduler.h:63: * \ingroup FdTbfqFfMacScheduler
On 2012/08/28 10:15:42, Nicola Baldo wrote:
> bug 1499 issue, see comment in fdbet-ff-mac-scheduler.h

Done.

http://codereview.appspot.com/6484049/diff/1/src/lte/model/fdtbfq-ff-mac-sche...
src/lte/model/fdtbfq-ff-mac-scheduler.h:68: 
On 2012/08/28 10:15:42, Nicola Baldo wrote:
> remove empty line

Done.

http://codereview.appspot.com/6484049/diff/1/src/lte/model/pss-ff-mac-schedul...
File src/lte/model/pss-ff-mac-scheduler.h (right):

http://codereview.appspot.com/6484049/diff/1/src/lte/model/pss-ff-mac-schedul...
src/lte/model/pss-ff-mac-scheduler.h:43: 
On 2012/08/28 10:15:42, Nicola Baldo wrote:
> doxygen

Done.

http://codereview.appspot.com/6484049/diff/1/src/lte/model/pss-ff-mac-schedul...
src/lte/model/pss-ff-mac-scheduler.h:59: * \ingroup PssFfMacScheduler
On 2012/08/28 10:15:42, Nicola Baldo wrote:
> bug 1499 issue, see comment in fdbet-ff-mac-scheduler.h

Done.

http://codereview.appspot.com/6484049/diff/1/src/lte/model/tdbet-ff-mac-sched...
File src/lte/model/tdbet-ff-mac-scheduler.h (right):

http://codereview.appspot.com/6484049/diff/1/src/lte/model/tdbet-ff-mac-sched...
src/lte/model/tdbet-ff-mac-scheduler.h:42: 
On 2012/08/28 10:15:42, Nicola Baldo wrote:
> doxygen

Done.

http://codereview.appspot.com/6484049/diff/1/src/lte/model/tdbet-ff-mac-sched...
src/lte/model/tdbet-ff-mac-scheduler.h:57: * \ingroup TdBetFfMacScheduler
On 2012/08/28 10:15:42, Nicola Baldo wrote:
> bug 1499 issue, see comment in fdbet-ff-mac-scheduler.h

Done.

http://codereview.appspot.com/6484049/diff/1/src/lte/model/tdmt-ff-mac-schedu...
File src/lte/model/tdmt-ff-mac-scheduler.h (right):

http://codereview.appspot.com/6484049/diff/1/src/lte/model/tdmt-ff-mac-schedu...
src/lte/model/tdmt-ff-mac-scheduler.h:49: * \ingroup TdMtFfMacScheduler
On 2012/08/28 10:15:42, Nicola Baldo wrote:
> bug 1499 issue, see comment in fdbet-ff-mac-scheduler.h

Done.

http://codereview.appspot.com/6484049/diff/1/src/lte/model/tdtbfq-ff-mac-sche...
File src/lte/model/tdtbfq-ff-mac-scheduler.h (right):

http://codereview.appspot.com/6484049/diff/1/src/lte/model/tdtbfq-ff-mac-sche...
src/lte/model/tdtbfq-ff-mac-scheduler.h:43: 
On 2012/08/28 10:15:42, Nicola Baldo wrote:
> doxygen

Done.

http://codereview.appspot.com/6484049/diff/1/src/lte/model/tdtbfq-ff-mac-sche...
src/lte/model/tdtbfq-ff-mac-scheduler.h:63: * \ingroup TdTbfqFfMacScheduler
On 2012/08/28 10:15:42, Nicola Baldo wrote:
> bug 1499 issue, see comment in fdbet-ff-mac-scheduler.h

Done.

http://codereview.appspot.com/6484049/diff/1/src/lte/model/tta-ff-mac-schedul...
File src/lte/model/tta-ff-mac-scheduler.h (right):

http://codereview.appspot.com/6484049/diff/1/src/lte/model/tta-ff-mac-schedul...
src/lte/model/tta-ff-mac-scheduler.h:49: * \ingroup TtaFfMacScheduler
On 2012/08/28 10:15:42, Nicola Baldo wrote:
> bug 1499 issue, see comment in fdbet-ff-mac-scheduler.h

Done.