I've been looking over this code, and actually I'm not sure if the fix I
proposed actually makes sense...



To edit my initial bug report, I'm not sure if the large SNR and small PER is
actually a problem.  Now I think that the interference calculations are
correct, except when delta=0.  More details are below...


So I think function (CalculateNoiseInterferenceW) introduces two areas of
strange behavior:

First, it calculates the SNR by determining the noise ONLY at the very
beginning that the packet is received.  I'm not really sure what this SNR goes
on to be used for, so I'm not sure if this is important or not.

Second, an NiChange event is pushed back at the start time of the event (the
start time that the packet is received).  This NiChange uses the
noiseInterference calculated at the start time of the event.  If the attached
program is run with delta=0, then noiseInterference is equal to the power of
the interfering packet.  If, however, delta is >0 (even .001), then the
interfering packet does not overlap with the start time of the intended packet,
and noiseInterference is equal to 0.  This creates a very sudden change in PER
from delta=0 and delta>0.  The problem here is that an interfering packet with
the same transmit time as the intended packet is actually counted as
overlapping twice, meaning there are two NiChange events added at the same
time, and both with the interfering packet's power.  To fix this, I'd just make
it so if the interfering packet 'overlaps' with the start time of the intended
packet, and the intended packet 'overlaps' with the start time of the
interfering packet, then only add to noiseInterference, and don't also push
back an NiChange event at that time.  The result looks like this:

            if((*i)->Overlaps (event->GetStartTime ()))
            {
                noiseInterference += (*i)->GetRxPowerW ();
            }         
            else if (event->Overlaps ((*i)->GetStartTime ())) 
            {
                ni->push_back (NiChange ((*i)->GetStartTime (),
(*i)->GetRxPowerW ()));
            }
            if (event->Overlaps ((*i)->GetEndTime ())) 
            {
                ni->push_back (NiChange ((*i)->GetEndTime (),
-(*i)->GetRxPowerW ()));
            }

a description of the intended behavior of the code can be found in:
http://www.cutebugs.net/files/wns2-yans.pdf

After looking at the yans paper, I do think my fix proposed in the first
comment does fix the bug in interference-helper.  Using the attached script,
run these two tests:

./waf --run "wifi-simple-interference --Irss=-80  --numPackets=200 --delta=1"

and:

./waf --run "wifi-simple-interference --Irss=-80  --numPackets=200 --delta=0"

And you'll see a difference in the amount of received packets (184 when delta=0
versus 200 when delta=1).

Changing the if statements in 'CalculateNoiseInterferenceW' to:

                        if((*i)->Overlaps (event->GetStartTime ()))
                        {
                                noiseInterference += (*i)->GetRxPowerW ();
                        }               
                        else if (event->Overlaps ((*i)->GetStartTime ())) 
                        {
                                ni->push_back (NiChange ((*i)->GetStartTime (),
(*i)->GetRxPowerW ()));
                        }
                        if (event->Overlaps ((*i)->GetEndTime ())) 
                        {
                                ni->push_back (NiChange ((*i)->GetEndTime (),
-(*i)->GetRxPowerW ()));
                        }

Makes it so the bug when the interference packet is received at the same time
as the intended packet, and therefore the noise is calculated twice for the
interference packet, is now fixed.

Now running:

./waf --run "wifi-simple-interference --Irss=-80  --numPackets=200 --delta=1"
./waf --run "wifi-simple-interference --Irss=-80  --numPackets=200 --delta=0"

Has little difference in packets received (or if there is one the difference is
by 1 or 2 packets received), as it should be.

I am not sure I understand the proposed fix. Can you provide a patch with "hg
diff" ?

Created an attachment (id=555) [details]
a potential patch ?

I believe that this patch reflects what you have in mind. I think I sort of
understand what bug it is fixing. 

What would be really really cool is if you could find a way to convert your
test program into a regression test added to interference-helper.cc (something
along the lines of dcf-manager-test.cc). If you can't do this, I guess I can
commit this patch if you confirm that it is what you expected but it's hard for
me to feel very good about this since none of our existing tests outline this
bug or change their output with or without the patch.

Hey,

Yeah, it looks like that patch does what I was talking about.  In the future I
will include diff files with my bug submissions.

Also, I'll look into converting this script into a regression test over the
next few days.  The reason this fix doesn't change any of the current test
outputs is that this only changes the output when the interfering packet and
the intended packet are received at exactly the same time (delta = 0 in this
script)

To explain the situation again, in the function that populates the niChange
array, (CalculateNoiseInterferenceW) we populate depending on whether the start
or end time of the interfering packet overlaps with the given event.  the
overlap method counts a given time as overlapping with an event if the given
time is within the range of the event, or equals the event's start or end time.
 When delta = 0, the intended packet arrives at the same time as the
interfering packet, the interfering packet is counted as 'overlapping' with the
intended packet, AND since the interfering packet contributes to the noise in
the system when the intended packet's start time occurs, the interfering
packet's noise is added to the 'noise interference at the start time' variable.
 You can see the error in the niChange array if you print out the members of
the array at the end of this function, because there will be two entries that
correspond to the interfering packet's power.  This means for the rest of the
duration of the intended signal, the interfering noise power will be too high
by a level equal to the interfering packet power.

(I hope that explanation makes sense, essentially if you look at the members of
the niChange array at the end of the CalculateNoiseInterferenceW function, it
should be apparent that there's something wrong...)

(In reply to comment #6)

> Also, I'll look into converting this script into a regression test over the
> next few days.  The reason this fix doesn't change any of the current test

Ok, I will wait to push this patch until we can add the test. Thanks a lot.

Created an attachment (id=581) [details]
Patch to add PER trace hook

Created an attachment (id=582) [details]
Test for interference bug

I just attached a test that detects this bug.  When run, it calculates three
interference tests, one when both packets are received at the same time, one
when the interference packet is received 1 microsecond after the intended
packet, and one when the interference packet is received 2 microseconds after
the intended packet.  Then, using the attached patch to trace the PER, it
calculates the difference in PER between the first and second test, and the
second and third test.  In the current implementation, these values should be
the same, so if they are the same, we 'pass', if they aren't, the test fails.  

Tom Henderson is going to transform this to a test in Craig's new framework, so
if the source seems a bit rough right now, that should be fixed.  If you have
any further comments tell me and I'll look into it/send updates to Tom.

+1 on the patch fixing this bug, and on the patch including the new PER trace
source.  I will sign up to make sure this test is included in the release if
the other two patches are pushed.

Okay after some thought I think I got what your problem is:

In case two packets have equal StartTimes, the power is added twice.

I agree with that, due to the <= in InterferenceHelper::Event::Overlap(). Your
patch fixes that using elseif, as obviously the <= should stay.

There should also be an equivalent situation at the packet end time due to the
>=. However, I believe it is irrelevant for further calculations.

So I also agree with the patch.

changeset: 7c3b105b6945

Since tom said that he would commit the test himself, I reassign this bug to
him.

(In reply to comment #13)
> changeset: 7c3b105b6945
> 
> Since tom said that he would commit the test himself, I reassign this bug to
> him.
> 

This was closed out in changeset 30f7ef5b318a