Bug 473 - [PATCH] Alternative ns-2 trace reader
[PATCH] Alternative ns-2 trace reader
Status: RESOLVED FIXED
Product: ns-3
Classification: Unclassified
Component: mobility models
pre-release
All All
: P5 normal
Assigned To: ns-bugs
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-01-19 05:19 EST by Francesco Malandrino
Modified: 2010-07-01 00:30 EDT (History)
11 users (show)

See Also:


Attachments
implementation (13.97 KB, text/x-c++src)
2009-01-19 05:19 EST, Francesco Malandrino
Details
header (3.10 KB, text/x-chdr)
2009-01-19 05:21 EST, Francesco Malandrino
Details
Changed the Ns2-mobility-helper.cc to fit the other Ns2-mobilty format (6.15 KB, text/x-c++src)
2010-02-05 04:36 EST, twaldecker
Details
Header for the ns-2-transmobilty (3.00 KB, text/x-chdr)
2010-02-05 04:38 EST, twaldecker
Details
Improvement and bug fixes of ns2-transmobility-helper.cc (9.41 KB, text/x-c++src)
2010-03-21 01:08 EDT, Martín Giachino
Details
Test suite for Ns2TransmobilityHelper + code style (18.57 KB, application/octet-stream)
2010-03-21 10:56 EDT, Pavel Boyko
Details
Rewrite code to ease maintenance, add fixes and test cases (28.04 KB, text/x-c++src)
2010-03-25 02:54 EDT, Martín Giachino
Details
File obsoleted by this one was not correct. (30.21 KB, text/x-c++src)
2010-03-26 19:56 EDT, Martín Giachino
Details
ns-2 mobility trace reader (18.30 KB, text/plain)
2010-03-29 21:10 EDT, Michael Nowatkowski
Details
header file for ns-2 mobility trace reader (3.86 KB, text/x-chdr)
2010-03-29 21:11 EDT, Michael Nowatkowski
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Francesco Malandrino 2009-01-19 05:19:54 EST
Created attachment 354 [details]
implementation
Comment 1 Francesco Malandrino 2009-01-19 05:21:02 EST
Created attachment 355 [details]
header
Comment 2 Francesco Malandrino 2009-01-19 05:24:14 EST
I have written a variant of the Ns2MobilityHelper, suitable to read ns-2
mobility traces like the following one (generated by the Random Trip
implementation, see http://lrcwww.epfl.ch/RandomTrip/ and
http://www.cs.rice.edu/~santa/research/mobility/).

$node_(0) set X_ 28.675920486450
$node_(0) set Y_ 13.803303718567
$node_(0) set Z_ 0.000000000000
$ns_ at 4.634906291962 "$node_(0) set X_ 28.675920486450"
$ns_ at 4.634906291962 "$node_(0) set Y_ 13.803303718567"
$ns_ at 4.634906291962 "$node_(0) set Z_ 0.000000000000"
$ns_ at 4.634906291962 "$node_(0) setdest 44.610576629639
52.860397338867 0.917085826397"
[omissis, full test trace attached]

It assumes the semantic of setdest to be "setdest dest_x, dest_y,
speed", i.e. it only applies to 2d model. However, it can read a wider
set of formats respect to the original ones, namely:

$node_(1) set X_ 50 #initial position
$ns_ at 4.634906291962 "$node_(0) set Y_ 13.803303718567" #schedule position
$ns_ at 55.137107849121 "$node_(0) setdest 65.125175476074
80.098602294922 1.097754716873" #schedule setdest

Additionally, it can skip the comments and the parsing seems to me
somewhat more robust than the original one.

I attach it here as it may be useful for someone else (put the files in
src/helper/, and remember to edit src/helper/wscript). Unfortunately, I
could not follow the ns-3 coding style (e.g. the properties should
become attributes, I guess). 
Comment 3 Mathieu Lacage 2009-01-20 08:12:07 EST
Are you saying that this implementation is able to parse a strict superset of what the current implementation parses ? If so, it would be immensively helpful to update this code to match the ns-3 coding style so that we can replace the existing implementation by your own implementation.
Comment 4 Francesco Malandrino 2009-01-31 15:17:12 EST
It parses a strict superset of the traces parsed by the current implementation, but it assumes a slightly different meaning.
More exactly, the three parameters of setdest are assumed to be x, y, velocity and not x, y, z.
Comment 5 Mathieu Lacage 2009-02-24 03:19:49 EST
(In reply to comment #4)
> It parses a strict superset of the traces parsed by the current implementation,
> but it assumes a slightly different meaning.
> More exactly, the three parameters of setdest are assumed to be x, y, velocity
> and not x, y, z.

Could we not make this class have a 'mode' setting to change the way it interprets the input file ?




Comment 6 Tom Henderson 2009-07-10 17:33:45 EDT
change subject so that it is flagged as a pending code contribution
Comment 7 Rikard Nordin 2009-11-20 04:41:57 EST
The implementation is using StaticSpeedMobilityModel, which is renamed to ConstantVelocityMobilityModel in current version. All StaticSpeed should be replaced by ConstantVelocity in both implementation and header.
Comment 8 Michael Nowatkowski 2009-12-18 19:53:26 EST
(In reply to comment #7)
> The implementation is using StaticSpeedMobilityModel, which is renamed to
> ConstantVelocityMobilityModel in current version. All StaticSpeed should be
> replaced by ConstantVelocity in both implementation and header.

This is from Francesco -- the code seems to work great!
=========
Dear Michael,

I didn't improve anymore the code, but I'm still using it, and it works correctly. This is how I use it in my simulation script:

    string mobi_trace = "/home/francesco/mobi/trace.tcl";
    Ns2WaypointMobilityHelper ag_mobi;
    ag_mobi.MobilityTraces().push_back(mobi_trace);
    ag_mobi.Install(wifiStaNodes);

If you feel like, could you post the above code as a comment to the bug?
This may be helpful to someone else.

Francesco
=========

This is a sample of the input file I used:
$node_(2) set X_ 930.142648
$node_(2) set Y_ 4428.890494
$ns_ at 0.010000 "$node_(2) setdest 940.000000 4780.000000 26.998299" # 
$node_(3) set X_ 904.717673
$node_(3) set Y_ 2965.750229
$ns_ at 0.010000 "$node_(3) setdest 898.000000 3284.000000 26.504635" # 
$ns_ at 13.020000 "$node_(3) setdest 865.539027 3494.580157 19.691406" # 
$ns_ at 14.020000 "$node_(2) setdest 968.626601 4830.891734 17.705093" # 
$ns_ at 17.317950 "$node_(2) setdest 1120.000000 5100.000000 15.671500" # 
$ns_ at 23.840324 "$node_(3) setdest 849.107327 3601.175547 21.259277" # 
$ns_ at 28.913612 "$node_(3) setdest 820.000000 3790.000000 18.904353" # 
$ns_ at 38.020000 "$node_(2) setdest 1071.492944 5170.894928 18.311784" #


This is a sample of the NS_LOG_DEBUG statements:
accepted line $node_(2) set X_ 930.142648: setx
accepted line $node_(2) set Y_ 4428.890494: sety
accepted line $ns_ at 0.010000 "$node_(2) setdest 940.000000 4780.000000 26.998299": sched move, end 13.02
accepted line $node_(3) set X_ 904.717673: setx
accepted line $node_(3) set Y_ 2965.750229: sety
accepted line $ns_ at 0.010000 "$node_(3) setdest 898.000000 3284.000000 26.504635": sched move, end 12.02
accepted line $ns_ at 13.020000 "$node_(3) setdest 865.539027 3494.580157 19.691406": sched move, end 23.8403
accepted line $ns_ at 14.020000 "$node_(2) setdest 968.626601 4830.891734 17.705093": sched move, end 17.3179
accepted line $ns_ at 17.317950 "$node_(2) setdest 1120.000000 5100.000000 15.671500": sched move, end 37.02
accepted line $ns_ at 23.840324 "$node_(3) setdest 849.107327 3601.175547 21.259277": sched move, end 28.9136
accepted line $ns_ at 28.913612 "$node_(3) setdest 820.000000 3790.000000 18.904353": sched move, end 39.02
accepted line $ns_ at 38.020000 "$node_(2) setdest 1071.492944 5170.894928 18.311784": sched move, end 42.711
Comment 9 twaldecker 2010-02-05 04:36:38 EST
Created attachment 753 [details]
Changed the Ns2-mobility-helper.cc to fit the other Ns2-mobilty format

Why is the source code from Francesco not in ns-3 yet?
My ns-2 mobilty traces are the same as Francescos.

I changed the existing ns-2-mobility because I didnt notice that the implementation from Francesco already exists.

This Line as example:
$ns_ at 13 "$node_(25) setdest 460.00 500.00 30.00"
is interpreted as:
time=13, nodeid=25, destination-x=460 destination-y=500, speed=30

I attach my changed files here.
Comment 10 twaldecker 2010-02-05 04:38:16 EST
Created attachment 754 [details]
Header for the ns-2-transmobilty

The Header file for my implementation

Thomas
Comment 11 Mathieu Lacage 2010-02-05 04:45:32 EST
(In reply to comment #9)
> Created an attachment (id=753) [details]
> Changed the Ns2-mobility-helper.cc to fit the other Ns2-mobilty format
> 
> Why is the source code from Francesco not in ns-3 yet?

Needs to be updated to match the coding style, among others. See the discussion above.
Comment 12 Martín Giachino 2010-03-21 01:08:16 EDT
Created attachment 797 [details]
Improvement and bug fixes of ns2-transmobility-helper.cc

Hi people,

I Changed de ns2-transmobility-helper.cc in order to correct some problems. The previous header file remains the same.

Changes:
--------

- Use of "model->GetPosition ()" was not correct. In this case, in which simulation is not running yet, invocation of model->GetPosition () always returns the initial position, so in this implementation, last position is maintained in a variable.

- As a consequence of the behavior of "model->GetPosition ()", speed and time was not calculated in the correct way, so it is corrected in this version.

- Added support for node pause.

- Add support for comment lines that begins with "#".

- Add support for lines like "$ns_ at 4.634906291962 "$node_(0) set X_ 28.675920486450""


I think this file is in ns3 coding style, and supports the same superset as the Francesco's one.

I tested this implementation using py-viz. Ns2 trace files that I used were generated by bonnmotion [1]. I tested it with traces generated for my own mobility model, who mixes more than one mobility model in one.


[1] http://net.cs.uni-bonn.de/wg/cs/applications/bonnmotion/
Comment 13 Pavel Boyko 2010-03-21 10:56:17 EDT
Created attachment 798 [details]
Test suite for Ns2TransmobilityHelper + code style
Comment 14 Pavel Boyko 2010-03-21 10:59:25 EDT
  Hi, Martin, 

  I'm happy to see these enhancements to the ns-2 mobility trace reader and will strongly support the new reader to be merged (replacing the current one) in the ns-3.9 time frame. 

  I see the following minimal requirements for that:

1. Support "original" 3D mode, where the last argument to setdest is interpreted as Z coordinate instead of speed. 

2. Write a (large) number of unit tests assuring that reader works as expected. I have started the test suite, please find it in the attached .cc file. Currently 3 tests pass, one ("square setdest") crashes and  another one ("scheduled set position") fails. To run test use:

./test.py -s mobility-ns2-trace-helper -v -t test.txt

and then cat test.txt to see testing results/errors. 

3. Current lexer/parser implementation looks very unreliable. I'd prefer to see more formal approach, at least to the lexer part, like Francisco did in his old patch. I'm afraid that maintaining current code will be very difficult in the future. M.b. you can use some code from that patch without changing the reader behavior? The good news is that having all these tests we can change the lexer/parser implementation later.

4. When all bugs will be fixed and a large number of nontrivial tests will pass, please submit new helper to the code review, as described here: http://www.nsnam.org/wiki/index.php/Developer_FAQ#Submitting_patches_for_consideration.

  Please let me know if you agree/disagree with this plan. I hope that you do volunteer some time to make ns-2 mobility trace reader useful and robust. 

  Best regards,
  Pavel

(In reply to comment #12)
> Created an attachment (id=797) [details]
> Improvement and bug fixes of ns2-transmobility-helper.cc
> 
> Hi people,
> 
> I Changed de ns2-transmobility-helper.cc in order to correct some problems. The
> previous header file remains the same.
> 
> Changes:
> --------
> 
> - Use of "model->GetPosition ()" was not correct. In this case, in which
> simulation is not running yet, invocation of model->GetPosition () always
> returns the initial position, so in this implementation, last position is
> maintained in a variable.
> 
> - As a consequence of the behavior of "model->GetPosition ()", speed and time
> was not calculated in the correct way, so it is corrected in this version.
> 
> - Added support for node pause.
> 
> - Add support for comment lines that begins with "#".
> 
> - Add support for lines like "$ns_ at 4.634906291962 "$node_(0) set X_
> 28.675920486450""
> 
> 
> I think this file is in ns3 coding style, and supports the same superset as the
> Francesco's one.
> 
> I tested this implementation using py-viz. Ns2 trace files that I used were
> generated by bonnmotion [1]. I tested it with traces generated for my own
> mobility model, who mixes more than one mobility model in one.
> 
> 
> [1] http://net.cs.uni-bonn.de/wg/cs/applications/bonnmotion/
Comment 15 Martín Giachino 2010-03-21 16:49:11 EDT
(In reply to comment #14)
>   Hi, Martin, 
> 
>   I'm happy to see these enhancements to the ns-2 mobility trace reader and
> will strongly support the new reader to be merged (replacing the current one)
> in the ns-3.9 time frame.

It sounds great!

 
>   I see the following minimal requirements for that:
> 
> 1. Support "original" 3D mode, where the last argument to setdest is
> interpreted as Z coordinate instead of speed.

I'm not agree with this, because in this case we will be changing semantics of ns2 setdes command [1]. I think that with set Z_ will be enough, as it is up to now.

 
> 2. Write a (large) number of unit tests assuring that reader works as expected.
> I have started the test suite, please find it in the attached .cc file.
> Currently 3 tests pass, one ("square setdest") crashes and  another one
> ("scheduled set position") fails. To run test use:
> 
> ./test.py -s mobility-ns2-trace-helper -v -t test.txt
> 
> and then cat test.txt to see testing results/errors.

Strange, I tested it with large simulations (I can attach it if necessary), with sqares (with Manhattan model) among others, and it works correctly. Anyway, I'll write more test cases if they are needed.

I'm new in ns3, and when I ran the command you said, i obtained this error message, suggests?:

------
Return code =  139
stderr =  assert failed. file=../src/simulator/default-simulator-impl.cc, line=185, cond="tAbsolute.IsPositive ()"
Segmentation fault

returncode = 139
---------- beign standard out ----------

---------- begin standard err ----------
assert failed. file=../src/simulator/default-simulator-impl.cc, line=185, cond="tAbsolute.IsPositive ()"
Segmentation fault

---------- end standard err ----------
CRASH: TestSuite mobility-ns2-trace-helper
0 of 1 tests passed (0 passed, 0 skipped, 0 failed, 1 crashed, 0 valgrind errors)
------



> 3. Current lexer/parser implementation looks very unreliable. I'd prefer to see
> more formal approach, at least to the lexer part, like Francisco did in his old
> patch. I'm afraid that maintaining current code will be very difficult in the
> future. M.b. you can use some code from that patch without changing the reader
> behavior? The good news is that having all these tests we can change the
> lexer/parser implementation later.

I think Ns2 traces format will never change, but on the other way your comment is true. I'll try to make this change as soon as possible.

 
> 4. When all bugs will be fixed and a large number of nontrivial tests will
> pass, please submit new helper to the code review, as described here:
> http://www.nsnam.org/wiki/index.php/Developer_FAQ#Submitting_patches_for_consideration.
> 
>   Please let me know if you agree/disagree with this plan. I hope that you do
> volunteer some time to make ns-2 mobility trace reader useful and robust.

I only have one question about test suite. Remember I'm new in ns3, so maybe I'm wrong, please correct me if is the case.

This ns2 helper is supposed to load ns2 trace from files so, I think we sould make test cases based on trace files and not using methods from the model class to generate waypoints.


>   Best regards,
>   Pavel

Well, I'll wait for your comments in order to define the plan, then I'll modify the code.

Thanks Pavel for the oportunity to be a ns3 contributor.


[1] http://www.isi.edu/nsnam/ns/doc/node174.html

Martín
Comment 16 Pavel Boyko 2010-03-22 09:48:46 EDT
  Hi, Martin, 

> > 1. Support "original" 3D mode, where the last argument to setdest is
> > interpreted as Z coordinate instead of speed.
> 
> I'm not agree with this, because in this case we will be changing semantics of
> ns2 setdes command [1]. I think that with set Z_ will be enough, as it is up to now.

  Please add link [1] to the class documentation. Actually I am not comfortable with breaking existing functionality in a such nonclear way. Mathieu, could you remember why did you treat that 3-d setdest parameter as Z? Do you know ns-3 users who use this semantics? (Here I hope that Mathieu will confess that he made a mistake and the question will be closed :).  

> Strange, I tested it with large simulations (I can attach it if necessary),
> with sqares (with Manhattan model) among others, and it works correctly.

  You were (un)lucky not to find really weird situation in the real simulation traces, that's why synthetic tests are so useful. 

> Anyway, I'll write more test cases if they are needed.

  Good. 

> I'm new in ns3, and when I ran the command you said, i obtained this error
> message, suggests?:

  This is how assertions implemented in ns-3. To debug it first enable core dumps:

$ ulimit -c unlimited

  then run test again

$ ./test.py -s mobility-ns2-trace-helper -v -t test.txt

  "core" file should be created in the current dir. Open it in debugger:

$ gdb ./build/debug/utils/test-runner core

  to see backtrace:

(gdb) bt
#0  0x00f5df28 in ns3::DefaultSimulatorImpl::Schedule (this=0x9a073b0, time=..., event=0x9a07738) at ../src/simulator/default-simulator-impl.cc:185
#1  0x00f4305e in ns3::Simulator::DoSchedule (time=..., impl=0x9a07738) at ../src/simulator/simulator.cc:243
#2  0x014fcd67 in ns3::Simulator::Schedule<void (ns3::ConstantVelocityMobilityModel::*)(ns3::Vector3D const&), ns3::Ptr<ns3::ConstantVelocityMobilityModel>, ns3::Vector3D> (time=..., mem_ptr=0x13c2396 <ns3::ConstantVelocityMobilityModel::SetVelocity(ns3::Vector3D const&)>, obj=..., a1=...)
    at debug/ns3/simulator.h:811
#3  0x014fee4a in ns3::Ns2TransmobilityHelper::LayoutObjectStore (this=0xbfb6dedc, store=...) at ../src/helper/ns2-transmobility-helper.cc:210
#4  0x01505b5d in ns3::Ns2TransmobilityHelper::Install<__gnu_cxx::__normal_iterator<ns3::Ptr<ns3::Node> const*, std::vector<ns3::Ptr<ns3::Node>, std::allocator<ns3::Ptr<ns3::Node> > > > > (this=0xbfb6dedc, begin=..., end=...) at ../src/helper/ns2-transmobility-helper.h:107
#5  0x015000af in ns3::Ns2TransmobilityHelper::Install (this=0xbfb6dedc) at ../src/helper/ns2-transmobility-helper.cc:277
#6  0x015049ef in ns3::Ns2TransmobilityHelperTest::DoRun (this=0x99d0f68) at ../src/helper/ns2-transmobility-helper.cc:430
#7  0x00e36cc1 in ns3::TestCase::Run (this=0x99d0f68) at ../src/core/test.cc:152
#8  0x00e38dc7 in ns3::TestSuite::DoRun (this=0x192f260) at ../src/core/test.cc:684
#9  0x00e384ee in ns3::TestSuite::Run (this=0x192f260) at ../src/core/test.cc:459
#10 0x0804a258 in main (argc=5, argv=0xbfb6e274) at ../utils/test-runner.cc:263

  You see that error occurs in ns2-transmobility-helper.cc line 210: 

if (time >= 0) Simulator::Schedule(Seconds(at + time),  &ConstantVelocityMobilityModel::SetVelocity, model, Vector(0, 0, 0));

  I guess that assertion says you that you try to schedule event to negative or infinite time, please check this. 

  All assertions in the tests can be debugged this way.
 
> I think Ns2 traces format will never change, but on the other way your comment
> is true. I'll try to make this change as soon as possible.

  As far as I know, bonnmotion guys have already extended mobility trace format to account for node failures and recoveries and they do want us to support this feature in ns-3 (with time). 
 
> This ns2 helper is supposed to load ns2 trace from files so, I think we sould
> make test cases based on trace files and not using methods from the 
> model class to generate waypoints.

  Test cases do both: read traces from files AND compare resulting mobility with build-in reference waypoints (see methods SetTrace() and AddReferencePoint()). The only trick is that traces are build-in in the code like this:

test->SetTrace ("$ns_ at 1.0 \"$node_(0) setdest 25 0 5\"");

and test case implementation dumps this string to temporary file and then read it as usual (and finally remove). This is done to keep test traces and reference waypoints for each test in the same place.

> Thanks Pavel for the oportunity to be a ns3 contributor.

  You are welcome. By the way submitting patch to review don't forget to update AUTHORS file.

  Best regards,
  Pavel
Comment 17 Michael Nowatkowski 2010-03-22 10:09:28 EDT
Thanks for continuing the work on the mobility trace readers!  Will this reader be able to read the traces at http://www.lst.inf.ethz.ch/research/ad-hoc/car-traces/, including turning on and off the wifi interface?  That would be a great help since these traces are quite expansive -- about 260,000 over a 21 hour period.

Thanks again!
Michael
Comment 18 Martín Giachino 2010-03-25 02:54:56 EDT
Created attachment 799 [details]
Rewrite code to ease maintenance, add fixes and test cases
Comment 19 Martín Giachino 2010-03-25 03:09:27 EDT
(In reply to comment #18)
> Created an attachment (id=799) [details]
> Rewrite code to ease maintenance, add fixes and test cases

Hi Pavel, in this version

- Add the link to the class documentation

- Write the parsing of the lines in a parser, similar to the Francesco

- Fix several cases in parser, in order to be robust for cases like: incomplete lines, malformed lines (with no brackets or just one), node id's number not being numbers and the same for x,y,z,speed and time, checks for time grater than cero, no negative speed, etc.

- Now your five test cases are working, and I added four more test.

Do you think it is ready to be submitted to code review? I'll be waiting for your response.

Thanks
Martín
Comment 20 Pavel Boyko 2010-03-25 05:53:54 EDT
  Hi, Martin,

  Thank you for fast update!

> - Now your five test cases are working, and I added four more test.

  Cool!
 
> Do you think it is ready to be submitted to code review? I'll be waiting for
> your response.

  Yes, I'm pretty sure that we should move to google code review tool to do some more review/update iterations and finally close this issue. So, please 

1. Correct coding style, you can use ./utils/check-style.py script for that in the way:

$ sudo apt-get install uncrustify # on ubuntu/debian
$ ./utils/check-style.py -l3 -f src/helper/ns2-transmobility-helper.cc

Unfortunately this has a side effect of replacing doxygen comments "///" to "// /", please replace them back in your text editor ;( Next, please configure your editor to expand tabs to spaces.

2. Add a simple usage example (examples/mobility folder can be created for that). Example should consist of simple well documented .cc script and a realistic .trace file. I want this example to discard samples/main-ns2-mob.cc in the future. Waf infrastructure can be copy-pasted from e.g. examples/mesh.

3. Start a code review issue as described here: http://www.nsnam.org/wiki/index.php/Developer_FAQ#Submitting_patches_for_consideration

4. Add code review link to this bug. 

I (and the other interesting people) will comment your code on appspot.

  Best regards,
  Pavel
Comment 21 Martín Giachino 2010-03-26 19:56:41 EDT
Created attachment 801 [details]
File obsoleted by this one was not correct.

The previous file that I've uploaded was not the correct one, this is it.

Sorry,
Martín
Comment 22 Martín Giachino 2010-03-27 02:26:05 EDT
(In reply to comment #21)

A code review was submitted, see http://codereview.appspot.com/802041

I couln't add a reference to the code review  in the wiki (http://www.nsnam.org/wiki/index.php/Reviews) because I do not have permission. Is it correct?

Martín
Comment 23 Pavel Boyko 2010-03-29 01:01:56 EDT
(In reply to comment #22)
> (In reply to comment #21)
> 
> A code review was submitted, see http://codereview.appspot.com/802041

  Ok, I'll try to review it asap. Before that -- please update review adding  helper's header, example  trace and diff to helper/wscript (something else?) It is very convenient to have all affected files in the review.

> I couln't add a reference to the code review  in the wiki
> (http://www.nsnam.org/wiki/index.php/Reviews) because I do not have permission.
> Is it correct?

  I guess ns-3.9 release manager will do this. 

  Pavel
Comment 24 Martín Giachino 2010-03-29 02:15:50 EDT
(In reply to comment #23)
> (In reply to comment #22)
> > (In reply to comment #21)
> > 
> > A code review was submitted, see http://codereview.appspot.com/802041
> 
>   Ok, I'll try to review it asap. Before that -- please update review adding 
> helper's header, example  trace and diff to helper/wscript (something else?) It
> is very convenient to have all affected files in the review.

Done.


> > I couln't add a reference to the code review  in the wiki
> > (http://www.nsnam.org/wiki/index.php/Reviews) because I do not have permission.
> > Is it correct?
> 
>   I guess ns-3.9 release manager will do this. 

Thanks, Tom Henderson did it.


Martín
Comment 25 aschenbruck 2010-03-29 20:29:11 EDT
Following Comment #17, I think it would be good to support turning on and off the nodes. 
There at least two reasons:
i) in reality, this may happen
ii) one of the core characteristics of MANETs it is to be able to deal with nodes leaving and (re-)joining the network

In BonnMotion (http://bonnmotion.net.cs.uni-bonn.de) we have currently one mobility model (the disaster area one) that switches on/off the nodes.

I think it would be great, if ns-3 could support this.

A node that is switched off should of course not send any messages on any layer.

If it is supported I think it would be a nice-to-have to support it with and without node reset. There may be scenarios where one wants to simulate a fixed number of nodes that are switched on and off. But there may also be scenarios where one wants to simulate that some nodes leave and other nodes do later join the network. If "switch on with reset" would also be supported, one could use the same nodes that were switched off before in the latter case.

Thanks for your work on mobility trace reading for ns-3,
best
Nils
Comment 26 Michael Nowatkowski 2010-03-29 21:08:56 EDT
I added to Francesco's files to turn on and off IPv4 adapters.  While this works for a specific case, I'm sure someone can figure out how to make it work for the general case.  The trace files mentioned in comment 17 do have entries that, by turning the interface on and off, basically allows nodes to enter and leave the simulation at any time (the nodes are all created at the start of the simulation, but don't interact unless the interface is turned on).

The interface on/off lines are at 182-224 in the attached file (ns2-waypoint-mobility-helper.cc).  I know this file doesn't meet the ns-3 coding style and there are some other coding no-no's in there, but like I said, it works for a specific case now, not the general case.  Hopefully it will be useful in some form to someone though.

Here are few lines from the trace file:

$ns_ at 0.0 "$node_(2) switch OFF" # set_X,Y,Z  
$node_(2) set X_ 0.000000
$node_(2) set Y_ 0.000000
$node_(2) set Z_ 0.0
$ns_ at 0.000000 "$node_(2) setdest 3940.046526 4116.298353 1000000000.000000" # init_node
$ns_ at 0.000100 "$node_(2) switch ON" # inside  
$ns_ at 0.000100 "$node_(2) setdest 4010.000000 4063.703704 12.713851" # 
$ns_ at 6.903802 "$node_(2) switch OFF" # leaving_area  

Michael
Comment 27 Michael Nowatkowski 2010-03-29 21:10:19 EDT
Created attachment 803 [details]
ns-2 mobility trace reader

modified file from Francesco's original
Comment 28 Michael Nowatkowski 2010-03-29 21:11:19 EDT
Created attachment 804 [details]
header file for ns-2 mobility trace reader
Comment 29 Pavel Boyko 2010-03-30 03:11:53 EDT
  Hi, 

(In reply to comment #25)
> Following Comment #17, I think it would be good to support turning on and off
> the nodes. 

  For me the task of introducing node or device failure/recovery models to ns-3 is much more generic than ns2 mobility traces reader. That's why I propose to leave this bug for the reader alone. In its current version the reader is not supposed to support any form of failure/recovery (just because ns-3 doesn't support this). It will be extremely easy to support _reading_ of node/interface on/off events _when_ the failure/recovery models will be designed and implemented in ns-3. 

  For now I suggest you to start new topic at the developer's list with the request of designing and implementing failure/recovery models in ns-3. I am sure that other people (including myself) have their own needs and requirements for failure modeling.   

> I think it would be great, if ns-3 could support this.

  I do agree.
 
> A node that is switched off should of course not send any messages on any
> layer.
> 
> If it is supported I think it would be a nice-to-have to support it with and
> without node reset. There may be scenarios where one wants to simulate a fixed
> number of nodes that are switched on and off. But there may also be scenarios
> where one wants to simulate that some nodes leave and other nodes do later join
> the network. If "switch on with reset" would also be supported, one could use
> the same nodes that were switched off before in the latter case.

  Please don't forget to copy-paste these use cases to the developer's list message mentioned above.

  Best regards,
  Pavel
Comment 30 Pavel Boyko 2010-03-30 03:30:02 EDT
  Hi, Michael, 

(In reply to comment #26)
> I added to Francesco's files to turn on and off IPv4 adapters.  

  I am pretty sure that copy-pasting one more implementation of the ns-2 mobility trace reader is a really bad idea. "The" ns-2 trace reader is reviewed here: http://codereview.appspot.com/802041 and it will be the only ns-2 trace reader. I will not test/review/merge the attached code. I do not recommend to use it either.

  Note, that shutting down an IP interface is quite different from node-level failure requested above. Also be aware the current implementation is quite limited, e.g. IP interface up/down doesn't cause the device shutdown/restart and some routing protocols do not respect it at all.

  Best regards,
  Pavel 

> While this
> works for a specific case, I'm sure someone can figure out how to make it work
> for the general case.  The trace files mentioned in comment 17 do have entries
> that, by turning the interface on and off, basically allows nodes to enter and
> leave the simulation at any time (the nodes are all created at the start of the
> simulation, but don't interact unless the interface is turned on).
> 
> The interface on/off lines are at 182-224 in the attached file
> (ns2-waypoint-mobility-helper.cc).  I know this file doesn't meet the ns-3
> coding style and there are some other coding no-no's in there, but like I said,
> it works for a specific case now, not the general case.  Hopefully it will be
> useful in some form to someone though.
> 
> Here are few lines from the trace file:
> 
> $ns_ at 0.0 "$node_(2) switch OFF" # set_X,Y,Z  
> $node_(2) set X_ 0.000000
> $node_(2) set Y_ 0.000000
> $node_(2) set Z_ 0.0
> $ns_ at 0.000000 "$node_(2) setdest 3940.046526 4116.298353 1000000000.000000"
> # init_node
> $ns_ at 0.000100 "$node_(2) switch ON" # inside  
> $ns_ at 0.000100 "$node_(2) setdest 4010.000000 4063.703704 12.713851" # 
> $ns_ at 6.903802 "$node_(2) switch OFF" # leaving_area  
> 
> Michael
Comment 31 Tom Henderson 2010-03-30 09:23:47 EDT
(In reply to comment #25)
> Following Comment #17, I think it would be good to support turning on and off
> the nodes. 
> There at least two reasons:
> i) in reality, this may happen
> ii) one of the core characteristics of MANETs it is to be able to deal with
> nodes leaving and (re-)joining the network
> 
> In BonnMotion (http://bonnmotion.net.cs.uni-bonn.de) we have currently one
> mobility model (the disaster area one) that switches on/off the nodes.
> 
> I think it would be great, if ns-3 could support this.
> 
> A node that is switched off should of course not send any messages on any
> layer.
> 
> If it is supported I think it would be a nice-to-have to support it with and
> without node reset. There may be scenarios where one wants to simulate a fixed
> number of nodes that are switched on and off. But there may also be scenarios
> where one wants to simulate that some nodes leave and other nodes do later join
> the network. If "switch on with reset" would also be supported, one could use
> the same nodes that were switched off before in the latter case.
> 
> Thanks for your work on mobility trace reading for ns-3,
> best
> Nils

I think we should discuss this issue outside of this thread (on the developers list), despite the fact that it possibly impacts the ability to support "node off/on" messages in the BonnMotion traces.
Comment 32 Pavel Boyko 2010-06-20 09:23:42 EDT
New reader by Martin merged, I close this bug.
Comment 33 Josh Pelkey 2010-06-21 15:38:49 EDT
(In reply to comment #32)
> New reader by Martin merged, I close this bug.

There are some valgrind errors due to this patch, specifically in mobility-ns2-trace-helper. One on line 145 is easily fixed by initializing id to zero.  The others appear to be related to iNodeId in ConfigNodesMovements.  I can't figure out why these errors are given though, since it looks to me like iNodeId is initialized just fine.
Comment 34 Josh Pelkey 2010-06-21 15:48:13 EDT
(In reply to comment #33)
> (In reply to comment #32)
> > New reader by Martin merged, I close this bug.
> 
> There are some valgrind errors due to this patch, specifically in
> mobility-ns2-trace-helper. One on line 145 is easily fixed by initializing id
> to zero.  The others appear to be related to iNodeId in ConfigNodesMovements. 
> I can't figure out why these errors are given though, since it looks to me like
> iNodeId is initialized just fine.

Sorry, the name of the test is mobility-ns2-trace-helper. The file is src/helper/ns2-mobility-helper.cc
Comment 35 Martín Giachino 2010-06-21 23:12:47 EDT
(In reply to comment #34)

Could you explain me how did you see errors related to iNodeId? When I ran the tests, all of them finished successfully, and i would like to reproduce them.

Thanks
Martín
Comment 36 Tom Henderson 2010-06-22 00:39:01 EDT
(In reply to comment #35)
> (In reply to comment #34)
> 
> Could you explain me how did you see errors related to iNodeId? When I ran the
> tests, all of them finished successfully, and i would like to reproduce them.
> 
> Thanks
> Martín

Try this:  

./waf --command-template="valgrind %s --basedir=`pwd` --suite=mobility-ns2-trace-helper" --run test-runner
Comment 37 Josh Pelkey 2010-06-23 13:41:38 EDT
(In reply to comment #35)
> (In reply to comment #34)
> 
> Could you explain me how did you see errors related to iNodeId? When I ran the
> tests, all of them finished successfully, and i would like to reproduce them.
> 
> Thanks
> Martín

Any updates on this one?  The test finishes successfully, but it will fail when run through valgrind.  In order to run tests through valgrind in test.py, you must use the -g or --grind option:

./test.py -g --suite=mobility-ns2-trace-helper

That will show the failure, but it won't give you any details.  In order to get the details, you have to run the test directly in valgrind, like Tom suggested:

./waf --command-template="valgrind %s --basedir=`pwd` --suite=mobility-ns2-trace-helper" --run test-runner
Comment 38 Martín Giachino 2010-06-23 14:37:13 EDT
(In reply to comment #37)
> (In reply to comment #35)
> > (In reply to comment #34)
> > 
> > Could you explain me how did you see errors related to iNodeId? When I ran the
> > tests, all of them finished successfully, and i would like to reproduce them.
> > 
> > Thanks
> > Martín
> 
> Any updates on this one?  The test finishes successfully, but it will fail when
> run through valgrind.  In order to run tests through valgrind in test.py, you
> must use the -g or --grind option:
> 
> ./test.py -g --suite=mobility-ns2-trace-helper
> 
> That will show the failure, but it won't give you any details.  In order to get
> the details, you have to run the test directly in valgrind, like Tom suggested:
> 
> ./waf --command-template="valgrind %s --basedir=`pwd`
> --suite=mobility-ns2-trace-helper" --run test-runner

OK, I will try to test it today.

Martín
Comment 39 Martín Giachino 2010-06-24 02:58:26 EDT
(In reply to comment #38)
> (In reply to comment #37)
> > (In reply to comment #35)
> > > (In reply to comment #34)
> > > 
> > > Could you explain me how did you see errors related to iNodeId? When I ran the
> > > tests, all of them finished successfully, and i would like to reproduce them.
> > > 
> > > Thanks
> > > Martín
> > 
> > Any updates on this one?  The test finishes successfully, but it will fail when
> > run through valgrind.  In order to run tests through valgrind in test.py, you
> > must use the -g or --grind option:
> > 
> > ./test.py -g --suite=mobility-ns2-trace-helper
> > 
> > That will show the failure, but it won't give you any details.  In order to get
> > the details, you have to run the test directly in valgrind, like Tom suggested:
> > 
> > ./waf --command-template="valgrind %s --basedir=`pwd`
> > --suite=mobility-ns2-trace-helper" --run test-runner
> 
> OK, I will try to test it today.
> 
> Martín


Well, I saw that the error is here:

          iNodeId = GetNodeIdInt (pr);
          if (iNodeId == -1)  <<< ERROR

but I'm confused too, because iNodeId is initialized with zero by default, and GetNodeIdInt function always returns an integer value. Any suggestions are welcome.

Martín
Comment 40 Tom Henderson 2010-06-27 23:52:55 EDT
Reopening until valgrind error clears.  I don't have time this evening to chase it down, but I have some more data on this.

The track-origins option to valgrind hints at what is the problem, at least for one of these:

./waf --command-template="valgrind --track-origins=yes %s --basedir=`pwd` --suite=mobility-ns2-trace-helper" --run test-runner

==18009== Conditional jump or move depends on uninitialised value(s)
==18009==    at 0x5E35CEB: ns3::Ns2MobilityHelper::ConfigNodesMovements(ns3::Ns2MobilityHelper::ObjectStore const&) const (ns2-mobility-helper.cc:199)
...
==18009==  Uninitialised value was created by a stack allocation
==18009==    at 0x5E34886: ns3::ParseNs2Line(std::string const&, ns3::ParseResult&) (ns2-mobility-helper.cc:291)
...
...
==18009== Conditional jump or move depends on uninitialised value(s)
==18009==    at 0x5E327FF: void ns3::Ns2MobilityHelper::Install<__gnu_cxx::__normal_iterator<ns3::Ptr<ns3::Node> const*, std::vector<ns3::Ptr<ns3::Node>, std::allocator<ns3::Ptr<ns3::Node> > > > >(__gnu_cxx::__normal_iterator<ns3::Ptr<ns3::Node> const*, std::vector<ns3::Ptr<ns3::Node>, std::allocator<ns3::Ptr<ns3::Node> > > >, __gnu_cxx::__normal_iterator<ns3::Ptr<ns3::Node> const*, std::vector<ns3::Ptr<ns3::Node>, std::allocator<ns3::Ptr<ns3::Node> > > >) const::MyObjectStore::Get(unsigned int) const (ns2-mobility-helper.h:118)
...
==18009==  Uninitialised value was created by a stack allocation
==18009==    at 0x5678420: ??? (in /home/tomh/hg/jun10/ns-3-allinone/ns-3-dev/build/debug/libns3.so)

The problem may be due to the following:  

          ParseResult pr = ParseNs2Line (line); // Parse line and obtain tokens

          // Check if the line corresponds with one of the three types of line
          if (pr.tokens.size () != 4 && pr.tokens.size () != 7 && pr.tokens.size () != 8)
            { ...


the variable pr is returned by ParseNs2Line and evaluated in the if () conditional; in the ParseNs2Line function, there is a stack allocation:
  ParseResult ret;

that is returned without any initialization if the trace line being read is malformed (which it is six times, in the case of the test program).
Comment 41 Pavel Boyko 2010-06-28 03:30:19 EDT
  I have pushed a fix, Tom -- could you check that now valgrind sees no errors and close this bug?

  On my system I still see strange memory leak:

$ ./waf --command-template="valgrind --track-origins=yes --leak-check=full %s --basedir=`pwd` --suite=mobility-ns2-trace-helper" --run test-runner

==12173== 120 bytes in 1 blocks are definitely lost in loss record 1 of 1
==12173==    at 0x4024F20: malloc (vg_replace_malloc.c:236)
==12173==    by 0x62AAF9D: getdelim (iogetdelim.c:68)
==12173==    by 0x657E2AA: ??? (in /lib/libselinux.so.1)
==12173==    by 0x658776C: ??? (in /lib/libselinux.so.1)
==12173==    by 0x65760F3: ??? (in /lib/libselinux.so.1)
==12173==    by 0x400DC4B: call_init (dl-init.c:70)
==12173==    by 0x400DD68: _dl_init (dl-init.c:134)
==12173==    by 0x400088E: ??? (in /lib/ld-2.11.1.so)
==12173== 
==12173== LEAK SUMMARY:
==12173==    definitely lost: 120 bytes in 1 blocks
==12173==    indirectly lost: 0 bytes in 0 blocks
==12173==      possibly lost: 0 bytes in 0 blocks
==12173==    still reachable: 0 bytes in 0 blocks
==12173==         suppressed: 0 bytes in 0 blocks
 
  but I hope this is just a valgrind whim.
Comment 42 Tom Henderson 2010-07-01 00:30:49 EDT
(In reply to comment #41)
>   I have pushed a fix, Tom -- could you check that now valgrind sees no errors
> and close this bug?
> 
>   On my system I still see strange memory leak:
> 
> $ ./waf --command-template="valgrind --track-origins=yes --leak-check=full %s
> --basedir=`pwd` --suite=mobility-ns2-trace-helper" --run test-runner
> 
> ==12173== 120 bytes in 1 blocks are definitely lost in loss record 1 of 1
> ==12173==    at 0x4024F20: malloc (vg_replace_malloc.c:236)
> ==12173==    by 0x62AAF9D: getdelim (iogetdelim.c:68)
> ==12173==    by 0x657E2AA: ??? (in /lib/libselinux.so.1)
> ==12173==    by 0x658776C: ??? (in /lib/libselinux.so.1)
> ==12173==    by 0x65760F3: ??? (in /lib/libselinux.so.1)
> ==12173==    by 0x400DC4B: call_init (dl-init.c:70)
> ==12173==    by 0x400DD68: _dl_init (dl-init.c:134)
> ==12173==    by 0x400088E: ??? (in /lib/ld-2.11.1.so)
> ==12173== 
> ==12173== LEAK SUMMARY:
> ==12173==    definitely lost: 120 bytes in 1 blocks
> ==12173==    indirectly lost: 0 bytes in 0 blocks
> ==12173==      possibly lost: 0 bytes in 0 blocks
> ==12173==    still reachable: 0 bytes in 0 blocks
> ==12173==         suppressed: 0 bytes in 0 blocks
> 
>   but I hope this is just a valgrind whim.

I did not see this now so I'm closing the bug again-- thanks for the fix.