[Ns-developers] Random Variables API changes

[Mathieu Lacage]

On Tue, 2009-01-13 at 17:42 -0500, Raj Bhattacharjea wrote:
> There were some changes to the RandomVariable API that were proposed for
> ns-3.3, but slipped.  The latest changes from Michele Weigle and Hadi Arbabi
> have been posted to a repository.  The relevent changeset is:
> 
> http://code.nsnam.org/raj/ns-3-rng-changes/rev/debf1a8a96d3
> 
> This is for an early first look at the new API.  I'll provide a summary and
> more details later.

1) You want to at least document a bit what SeedManager does. i.e., it
provides seeds for every other random variable.

2) I am fairly certain that this method should go away because it can't
possibly be implemented correctly:
	static uint32_t SeedManager::GetSeed ();

Say, I call SetSeed (0, 1, 2, 3, 4, 5), what is the output of
GetSeed(void) ?

I know that I suggested it in the first place :)

3) From an implementation perspective:

I wonder why you don't just forward all the SeedManager::Set/Get method
calls to the RngStream class rather than keep track of your own global
see: it would save you from having to keep your own set of static global
seeds (the same variables are stored in RngStream), you could move
the ::Initialize code below to RngStream::RngStream and remove the call
to ::Initialize from all the GetValue methods.



169 void RandomVariableBase::Initialize()
170 { 
171   uint32_t s[6];
172   if (RandomVariableBase::initialized) return; // Already initialized and seeded

coding style: 
if ()
  {
    //
  }

173   RandomVariableBase::initialized = true;
174   SeedManager::GetSeed(s);
175   RngStream::SetPackageSeed(s);
176 }


303 double UniformVariableImpl::GetValue(double s, double l) 
304 {

No need to check ::initialized before calling Initialize because Initialize checks initialized first

305 	  if(!RandomVariableBase::initialized)
306 	  {
307 	    RandomVariableBase::Initialize();
308 	  }
309 	  if(!m_generator)
310 	  {
311 	    m_generator = new RngStream();
312 	    m_generator->InitializeStream();
313 	    m_generator->ResetNthSubstream(SeedManager::GetRun());
314 	  }
315 	  return s + m_generator->RandU01() * (l-s); 
316 }

4) From an implementation perspective, I think that you are missing the
NS_RNG env variable handling and the various --Rng* command line options
I suggested in:
http://mailman.isi.edu/pipermail/ns-developers/2008-November/004907.html

Anyway, this tree looks much better than the current API and the
previous patch :) Thanks a lot for working on this.

regards,
Mathieu