Burst Error Model

Burst Error Model 

The BurstErrorModel determines which bursts of packets are flagged as errored corresponding to an underlying distribution, burst-event error rate, and burst size.  Error events are decided based on a uniformly distributed random variable called the decision parameter (m_ranVariable).  Every transmission of a packet triggers a comparison between the decision variable and the error rate (m_burstRate).  A secondary decision variable indicates a new error event during which upcoming packets will be errored until the number of dropped packets equals to the burst size or until the newly generated decision variable is again smaller than the error rate.  Similar to the decision parameter, in this model, the burst size (m_burstSize) is another uniformly distributed random variable within a tunable interval.

[Tom Henderson, 2013-03-20 20:31:39]

The main comments that need to be addressed are in the error-model.cc
implementation:

- check operation of DoCorrupt(), and document
- merge the AssignStreams() methods unless a strong use case to separate these
exists
- implement DoReset()

Other issues are more cosmetic but please ack or discuss whether you agree to
the comments.

Since this model was submitted, there was some documentation added to ns-3-dev
(src/network/doc/error-models.rst) and it would be nice to extend this also
before the release.

https://codereview.appspot.com/7183044/diff/1/examples/error-model/burst-erro...
File examples/error-model/burst-error-model.cc (right):

https://codereview.appspot.com/7183044/diff/1/examples/error-model/burst-erro...
examples/error-model/burst-error-model.cc:42: // - Tracing to file
"burst-error-model.tr"
This is very similar to simple-error-model.cc.  Could it instead just be added
as an option to that example (switch on the type of error model via command line
argument?).  That would reduce the need to repeatedly compile an additional
example that is very similar to another one.

https://codereview.appspot.com/7183044/diff/1/src/network/utils/error-model.cc
File src/network/utils/error-model.cc (right):

https://codereview.appspot.com/7183044/diff/1/src/network/utils/error-model.c...
src/network/utils/error-model.cc:83: */
Similar comment to the one I made in the header file; the above should be
deleted.

https://codereview.appspot.com/7183044/diff/1/src/network/utils/error-model.c...
src/network/utils/error-model.cc:320: .AddAttribute ("RanVar", "The decision
variable attached to this error model.",
"BurstStart" might be more descriptive name

https://codereview.appspot.com/7183044/diff/1/src/network/utils/error-model.c...
src/network/utils/error-model.cc:325: StringValue
("ns3::UniformRandomVariable[Min=1|Max=4]"),
does the value 4 ever get reached (when GetInteger() is called)?

https://codereview.appspot.com/7183044/diff/1/src/network/utils/error-model.c...
src/network/utils/error-model.cc:335: NS_LOG_FUNCTION_NOARGS ();
We do not use this variant of logging in non-static functions; we use instead
NS_LOG_FUNCTION (this);

https://codereview.appspot.com/7183044/diff/1/src/network/utils/error-model.c...
src/network/utils/error-model.cc:354: m_burstRate = rate;
strictly speaking, these Set/Get are not necessary (the
SetAttribute/GetAttribute can be used) but I am OK with leaving them in for
alignment with existing error-model.cc that does this.

https://codereview.appspot.com/7183044/diff/1/src/network/utils/error-model.c...
src/network/utils/error-model.cc:395: double ranVar = m_ranVariable
->GetValue();
I am not sure the logic of this is correct.  It seems like you want to model a
two-state process, either within a burst or outside of a burst.  When outside a
burst, you are determining, on a per-packet basis, whether it is time to go into
a burst loss.  However, once in a burst, you probably do not want to consult
m_ranVariable, but instead, this code does not seem to check first whether we're
in a burst event (checking m_counter first) but instead always gets a new ranVar
value. 

I expected to see something like:

if (m_counter < m_currentBurstSz)
  // increment m_counter, return true
else if (m_ranVariable->GetValue() < m_burstRate)
  // start a new burst
else
  return false;

How does this model relate to the classical Gilbert Elliott model for burst
errors?

Can you please clarify (in the doxygen to this class) what is the desired model
for entering/leaving burst events, and align this code?

https://codereview.appspot.com/7183044/diff/1/src/network/utils/error-model.c...
src/network/utils/error-model.cc:423: /* re-initialize any state; no-op for now
*/
Shouldn't this reset to an initialized state (if you are in a burst state, exit
the burst state)?

https://codereview.appspot.com/7183044/diff/1/src/network/utils/error-model.h
File src/network/utils/error-model.h (right):

https://codereview.appspot.com/7183044/diff/1/src/network/utils/error-model.h...
src/network/utils/error-model.h:69: * Foundation, Inc., 59 Temple Place, Suite
330, Boston, MA  02111-1307  USA
there is no need to add another GPL statement, and also, I prefer to avoid long
attributions and research funding notes in the header.  A single sentence like
"Burst error model added by Truc Anh N. Nguyen <annguyen@ittc.ku.edu>" would be
sufficient.

https://codereview.appspot.com/7183044/diff/1/src/network/utils/error-model.h...
src/network/utils/error-model.h:266: * the default for the decision variable is
to use a Uniform(0,1) distribution;
please clarify what is the decision variable (i.e. whether a burst starts on
this packet or not)

https://codereview.appspot.com/7183044/diff/1/src/network/utils/error-model.h...
src/network/utils/error-model.h:308: int64_t AssignRanVarStreams (int64_t
stream);
Why do you want to fix one to a stream but leave the other unassigned?  I don't
see the use case for this, so I would suggest to collapse this and the next
method into a single "AssignStreams()"

https://codereview.appspot.com/7183044/diff/1/src/test/burst-error-model-test...
File src/test/burst-error-model-test-suite.cc (right):

https://codereview.appspot.com/7183044/diff/1/src/test/burst-error-model-test...
src/test/burst-error-model-test-suite.cc:31: */
This can just be added as a separate test case to the existing test suite (which
I recently moved to src/network/test)