[Ns-developers] 802.11 first changesets to comment on.

[Timo Bingmann]

On Friday 28 November 2008 08:32:38 Mathieu Lacage wrote:
> hi timo,
> 
> On Thu, 2008-11-27 at 15:53 +0100, Timo Bingmann wrote:
> 
> > I've finished catching up on the wifi changes.
> > It would be great, if you could give me a quick review or hints on the
> > following changesets, which I've put into the hg repo at
> > http://idlebox.net/2008/ns-3-wifi/code/ns-3-wifiex/
> 
> Ok, quick initial comments:

Ok, I'll clarify some aspects.

> > Brief change description:
> > - NakagamiPropagationLossModel added.
> > - Required for that: GammaVariable and ErlangVariable random
> > variables.
> 
> Both look good to me. You will need approval from the random variable
> maintainer, that is, michele weigle: just create a bug report and attach
> a link to all relevant changesets:
> http://idlebox.net/2008/ns-3-wifi/code/ns-3-wifiex/rev/4cc0bd0187f2
> http://idlebox.net/2008/ns-3-wifi/code/ns-3-wifiex/rev/c2cc41757d7b
> http://idlebox.net/2008/ns-3-wifi/code/ns-3-wifiex/rev/a2c275b23a7e

Will do that.

> > - Change to PropagationLossModel::GetLoss() parameters. Required
> > because Nakagami envelopes the absolute rxPower. It's not
> > multiplicative.
> 
> Adding an extra parameter to the method sounds fine to me. However, the
> DoGetLoss method was expected previously to return one of the components
> of the final loss, not the final loss and that loss was then used by
> PropagationLossModel::GetLoss to calculate the final aggregate loss with
> the 'next' propagation loss model. Making the DoGetLoss method return
> the final loss kills the whole idea of iterating over the list of loss
> models to add their losses, thus, kills the idea of having a list of
> loss models. Thus, while I understand that you might need an extra
> parameter to DoGetLoss, and GetLoss, changing the semantics of its
> return value seem really wrong to me.

Yes, previously it was like taking txPowerDb and adding/subtracting the result of GetLoss(a,b). Thus GetLoss never knows the absolute power, which it doesn't need to because dB is logarithmic and the two propagation models sum up to a multiplicative loss.

Nakagami however uses the absolute power value as a RV parameter. So I need to add the current txPowerDb to the parameter list, and have DoGetLoss() return rxPowerDb. Which then can be passed into the next propagation model. So the m_next changes from adding up dB values to a function composition.
Old was rxPowerDb = txPowerDb + DoGetLoss1(a,b) + DoGetLoss2(a,b).
New is rxPowerDb = DoGetLoss2( DoGetLoss1(txPowerDb, a,b), a,b);

> > - Rewrote samples/main-random-variables to output all implemented RVs.
> 
> Looks pretty good. Will also require michele weigle approval.

Problem is that it depends on the Gnuplot extensions.

> > - Added samples/main-propagation-loss.cc, which outputs a plot of all
> > loss models (except Jakes', which I couldnt get to work right or don't
> > understand it.)
> 
> Looks pretty good: I don't know much about the Jakes model because I
> never used it myself. I will try to see if I can find a simulation
> script written by federico which uses it.
> 
> > - Lots of changes to the Gnuplot classes to make the two plot programs
> > work.
> 
> 1) I liked the ability to not have to allocate objects on the heap: is
> there a specific reason why you made GnuplotDataset derive from
> RefCountBase ? I am not saying that you should not: I am just trying to
> understand the rationale.

Those changes are aimed at creating experiment functions like:

Ptr<GnuplotDataset> RunExperimentToGenerateOneFunctionLine(Ptr<Object> param, int a, ...)

Using std::vector<Data*> inside GnuplotDataset means copying all the dataset's values. So I needed to add reference counting to GnuplotDataset. I did one version with internal (hand-coded) reference counting, but obviously using ns-3's RefCount or even Object is better.

Same goes then for GnuplotCollection holding a vector of Gnuplot.

> 2) what is the difference between a dataset title and a dataset
> function ? (i.e., what is the purpose of GnuplotDataset::SetFunction).
> it looks unused.

The SetFunction is used to plot non-datapoint line. E.g. the RV probability density functions in the RV plot.

> 3) it would be cool to make DetectTerminal and SetTerminal private and
> call them from the Gnuplot constructor.

SetTerminal() can be used after construction to add more terminal options like "pdf enhanced size 5.0,3.0 ...".

> 4) I would tend to avoid the duplicate functionality provided by void
> AddDataset (Ptr<GnuplotDataset> dataset, const std::string& title); and
> just ask the user to set the title on the dataset himself.

Useful for:
gnuplot.AddDataset( RunExperimentToGenerateOneFunctionLine(param1), "With Param 1" );
gnuplot.AddDataset( RunExperimentToGenerateOneFunctionLine(param2), "With Param 2" );

> 5) What do you use extra for ?

Well gnuplot has so many modifiers and text comments, I doubt it would be useful to create a c++ representation of each. Thus AddExtra can be used to add any of those formatting commands.
Eg. it is a was of setting "xrange", "xtics", .. etc or in a GnuplotDataset context adding "linecolor 3 pointsize 5"...

> 6) GnuplotDataset::AddEmptyLine is totally evil. Can't you figure out a
> way to get rid of it ?

No, I've created you a plot of the 3d surface without the empty lines. It is unreadable. See
ns-3-main-propagation-loss-withoutEmptyLines.pdf
at http://idlebox.net/2008/ns-3-wifi/results/

> 7) GnuplotDataset::Type looks also pretty evil: would it not make sense
> to simply create an unrelated Gnuplot3dDataset class and make the
> Gnuplot class be able to also handle it ? This might help with (6)
> above.

Yes, but it seemed overkill at that moment. However right now I believe making 4 classes is better: GnuplotDataset, GnuplotFunction, Gnuplot3dDataset and then a abstract class GnuplotLine (or different name?).

> > You can find the generated pdf plots at
> > http://idlebox.net/2008/ns-3-wifi/results/
> 
> these do look pretty cool.
> 
> > 
> > The output of Nakagami is cross-checked against ns-2. See the two
> > plots: yes, one is with ns-2, the other with ns-3.
> > 
> > One conceptional idea: the code for NakagamiPropagationLossModel is
> > taken from ns-2. It actually contains two propagation loss models: a
> > 3-piece log distance loss model followed by a 3-piece Nakagami fast
> > fading model. Conceptionally it would be better to separate them: use
> > LogDistanceLossModel followed by the Nakagami model using the chaining
> > feature. However this would require me to extend LogDistanceLossModel
> > into the 3-piecewise composition. It introduces more complexity. Maybe
> > just add a new class ThreeLogDistanceLossModel? What do you think?
> 
> This, indeed, seems like the right thing to do.

Yes. If it's ok to change LogDistanceLossModel into one with 3 distance ranges and different exponents for each.

> > 
> > Just tell me if I'm on the right track.
> 
> you seem to be doing great :)
> 
> Mathieu

Greetings
Timo