Bug 2647

Summary: ideal-wifi-manager-example crashes when NSS > 1
Product: ns-3 Reporter: sebastien.deronne
Component: wifiAssignee: Matías Richart <matis18>
Status: RESOLVED FIXED    
Severity: major CC: matis18, ns-bugs, redieteab.orange, tomh
Priority: P2    
Version: ns-3-dev   
Hardware: All   
OS: All   
Attachments: Select the nss according to the selected mode
alternative patch
Fix bug and include checking for SS support at the receiver
Patch to test different SS at sender and receiver in ideal
Fix bug and include checking for SS support at the receiver
Patch to extend ideal example
example to replace ideal and minstrelht examples

Description sebastien.deronne 2017-02-01 05:30:36 UTC
I am adding ideal-wifi-manager-example to regression and ideal wifi manager tests are crashing.

List of CRASHed tests:
    src/wifi/examples/ideal-wifi-manager-example --standard=802.11n-2.4GHz --channelWidth=20 --shortGuard=0 --nss=2
    src/wifi/examples/ideal-wifi-manager-example --standard=802.11n-2.4GHz --channelWidth=20 --shortGuard=0 --nss=3
    src/wifi/examples/ideal-wifi-manager-example --standard=802.11n-2.4GHz --channelWidth=20 --shortGuard=0 --nss=4
    src/wifi/examples/ideal-wifi-manager-example --standard=802.11n-2.4GHz --channelWidth=20 --shortGuard=1 --nss=2
    src/wifi/examples/ideal-wifi-manager-example --standard=802.11n-2.4GHz --channelWidth=20 --shortGuard=1 --nss=3
    src/wifi/examples/ideal-wifi-manager-example --standard=802.11n-2.4GHz --channelWidth=20 --shortGuard=1 --nss=4
    src/wifi/examples/ideal-wifi-manager-example --standard=802.11n-2.4GHz --channelWidth=40 --shortGuard=0 --nss=2
    src/wifi/examples/ideal-wifi-manager-example --standard=802.11n-2.4GHz --channelWidth=40 --shortGuard=0 --nss=3
    src/wifi/examples/ideal-wifi-manager-example --standard=802.11n-2.4GHz --channelWidth=40 --shortGuard=0 --nss=4
    src/wifi/examples/ideal-wifi-manager-example --standard=802.11n-2.4GHz --channelWidth=40 --shortGuard=1 --nss=2
    src/wifi/examples/ideal-wifi-manager-example --standard=802.11n-2.4GHz --channelWidth=40 --shortGuard=1 --nss=3
    src/wifi/examples/ideal-wifi-manager-example --standard=802.11n-2.4GHz --channelWidth=40 --shortGuard=1 --nss=4
    src/wifi/examples/ideal-wifi-manager-example --standard=802.11n-5GHz --channelWidth=20 --shortGuard=0 --nss=2
    src/wifi/examples/ideal-wifi-manager-example --standard=802.11n-5GHz --channelWidth=20 --shortGuard=0 --nss=3
    src/wifi/examples/ideal-wifi-manager-example --standard=802.11n-5GHz --channelWidth=20 --shortGuard=0 --nss=4
    src/wifi/examples/ideal-wifi-manager-example --standard=802.11n-5GHz --channelWidth=20 --shortGuard=1 --nss=2
    src/wifi/examples/ideal-wifi-manager-example --standard=802.11n-5GHz --channelWidth=20 --shortGuard=1 --nss=3
    src/wifi/examples/ideal-wifi-manager-example --standard=802.11n-5GHz --channelWidth=20 --shortGuard=1 --nss=4
    src/wifi/examples/ideal-wifi-manager-example --standard=802.11n-5GHz --channelWidth=40 --shortGuard=0 --nss=2
    src/wifi/examples/ideal-wifi-manager-example --standard=802.11n-5GHz --channelWidth=40 --shortGuard=0 --nss=3
    src/wifi/examples/ideal-wifi-manager-example --standard=802.11n-5GHz --channelWidth=40 --shortGuard=0 --nss=4
    src/wifi/examples/ideal-wifi-manager-example --standard=802.11n-5GHz --channelWidth=40 --shortGuard=1 --nss=2
    src/wifi/examples/ideal-wifi-manager-example --standard=802.11n-5GHz --channelWidth=40 --shortGuard=1 --nss=3
    src/wifi/examples/ideal-wifi-manager-example --standard=802.11n-5GHz --channelWidth=40 --shortGuard=1 --nss=4

./waf --run "ideal-wifi-manager-example --standard=802.11n-2.4GHz --channelWidth=20 --shortGuard=0 --nss=2
msg="MCS value does not match NSS value: MCS = 7, NSS = 2", file=../src/wifi/model/wifi-phy.cc, line=2471

The same tests are passing fine with Minstrel HT.
Comment 1 sebastien.deronne 2017-02-01 05:33:04 UTC
I guess this can be assigned to Tom :-)

Please note that I still need to deliver a small fix to handle guard interval in nanoseconds in ideal wifi manager. I will deliver the patch later today.
Comment 2 Matías Richart 2017-02-01 13:14:10 UTC
Created attachment 2771 [details]
Select the nss according to the selected mode

The problem is that the nss used does not always correspond to the mode selected.
I attach a patch.
Comment 3 sebastien.deronne 2017-02-01 15:24:25 UTC
(In reply to Matías Richart from comment #2)
> Created attachment 2771 [details]
> Select the nss according to the selected mode
> 
> The problem is that the nss used does not always correspond to the mode
> selected.
> I attach a patch.

Thanks, it's working like a charm :-)
Is there absolutely a need to keep 2 variables to store nss?
Comment 4 Matías Richart 2017-02-02 10:20:49 UTC
(In reply to sebastien.deronne from comment #3)
> (In reply to Matías Richart from comment #2)
> > Created attachment 2771 [details]
> > Select the nss according to the selected mode
> > 
> > The problem is that the nss used does not always correspond to the mode
> > selected.
> > I attach a patch.
> 
> Thanks, it's working like a charm :-)
> Is there absolutely a need to keep 2 variables to store nss?

It is not. A uploaded an alternative.
Comment 5 Matías Richart 2017-02-02 10:21:08 UTC
Created attachment 2772 [details]
alternative patch
Comment 6 sebastien.deronne 2017-02-02 11:00:21 UTC
(In reply to Matías Richart from comment #5)
> Created attachment 2772 [details]
> alternative patch

I did not check whether this works, but I have 2 questions:
1/ We do this change already a bit above in the code, why this is needed once again there?
2/ I see from ideal that we pick the highest supported tx spatial stream of the transmitter without checking what the receiver does support. Is this correct? Was this ever tested against a 4 Tx emitter and a 3 Rx receiver? I see this also in your change and I think the case of unequal Tx-Rx pairs is not taken into account.
Comment 7 Matías Richart 2017-02-02 11:27:03 UTC
(In reply to sebastien.deronne from comment #6)
> (In reply to Matías Richart from comment #5)
> > Created attachment 2772 [details]
> > alternative patch
> 
> I did not check whether this works, but I have 2 questions:
> 1/ We do this change already a bit above in the code, why this is needed
> once again there?

The code I added is to get the nss of the "maxMode" selected in the loop. Which doesn't necessarily matches the last nss picked in the loop.

> 2/ I see from ideal that we pick the highest supported tx spatial stream of
> the transmitter without checking what the receiver does support. Is this
> correct? Was this ever tested against a 4 Tx emitter and a 3 Rx receiver? I
> see this also in your change and I think the case of unequal Tx-Rx pairs is
> not taken into account.

You are right, it will fail.
I can work on that.
Comment 8 sebastien.deronne 2017-02-02 11:31:19 UTC
(In reply to Matías Richart from comment #7)
> (In reply to sebastien.deronne from comment #6)
> > (In reply to Matías Richart from comment #5)
> > > Created attachment 2772 [details]
> > > alternative patch
> > 
> > I did not check whether this works, but I have 2 questions:
> > 1/ We do this change already a bit above in the code, why this is needed
> > once again there?
> 
> The code I added is to get the nss of the "maxMode" selected in the loop.
> Which doesn't necessarily matches the last nss picked in the loop.

Ok sorry, my fault.

> 
> > 2/ I see from ideal that we pick the highest supported tx spatial stream of
> > the transmitter without checking what the receiver does support. Is this
> > correct? Was this ever tested against a 4 Tx emitter and a 3 Rx receiver? I
> > see this also in your change and I think the case of unequal Tx-Rx pairs is
> > not taken into account.
> 
> You are right, it will fail.
> I can work on that.

Thanks. Not sure there is an obvious call, I am not sure we store the supported number of streams so far. If you miss time let me know and I'll add this.
Comment 9 Matías Richart 2017-02-02 14:47:47 UTC
Created attachment 2773 [details]
Fix bug and include checking for SS support at the receiver

The patch proposes to try all possible NSSs in VHT that are also supported by the receiver.
It also includes the fix for bug 2606.

I'll also attach a test patch.
Comment 10 Matías Richart 2017-02-02 14:50:20 UTC
Created attachment 2774 [details]
Patch to test different SS at sender and receiver in ideal

A simple patch to try the new proposal.
We should extend the example to test all possible combinations.

My test: ./waf --run "ideal-wifi-manager-example --standard=802.11n-5GHz --channelWidth=20 --shortGuard=0 --nss=4"
Comment 11 sebastien.deronne 2017-02-02 15:07:02 UTC
(In reply to Matías Richart from comment #10)
> Created attachment 2774 [details]
> Patch to test different SS at sender and receiver in ideal
> 
> A simple patch to try the new proposal.
> We should extend the example to test all possible combinations.
> 
> My test: ./waf --run "ideal-wifi-manager-example --standard=802.11n-5GHz
> --channelWidth=20 --shortGuard=0 --nss=4"

Thanks.
I do not understand why you do those checks for VHT only?

FYI I've pushed your fix for bug 2606.
Comment 12 Matías Richart 2017-02-02 15:41:03 UTC
(In reply to sebastien.deronne from comment #11)
> (In reply to Matías Richart from comment #10)
> > Created attachment 2774 [details]
> > Patch to test different SS at sender and receiver in ideal
> > 
> > A simple patch to try the new proposal.
> > We should extend the example to test all possible combinations.
> > 
> > My test: ./waf --run "ideal-wifi-manager-example --standard=802.11n-5GHz
> > --channelWidth=20 --shortGuard=0 --nss=4"
> 
> Thanks.
> I do not understand why you do those checks for VHT only?

Do you mean the NSS check?
In HT, we have a different mode indexes for each value of NSS. So, as we use GetMcsSupported (station) we know the mode (and NSS) is supported by the receiver.

In VHT, the same mode index can identify different NSSs, so I have to try all possible NSSs and check if the receiver supports them.

> 
> FYI I've pushed your fix for bug 2606.

Great!
Comment 13 sebastien.deronne 2017-02-02 15:42:46 UTC
(In reply to Matías Richart from comment #12)
> (In reply to sebastien.deronne from comment #11)
> > (In reply to Matías Richart from comment #10)
> > > Created attachment 2774 [details]
> > > Patch to test different SS at sender and receiver in ideal
> > > 
> > > A simple patch to try the new proposal.
> > > We should extend the example to test all possible combinations.
> > > 
> > > My test: ./waf --run "ideal-wifi-manager-example --standard=802.11n-5GHz
> > > --channelWidth=20 --shortGuard=0 --nss=4"
> > 
> > Thanks.
> > I do not understand why you do those checks for VHT only?
> 
> Do you mean the NSS check?
> In HT, we have a different mode indexes for each value of NSS. So, as we use
> GetMcsSupported (station) we know the mode (and NSS) is supported by the
> receiver.
> 
> In VHT, the same mode index can identify different NSSs, so I have to try
> all possible NSSs and check if the receiver supports them.
> 

Indeed, thanks.
Can't we imagine to have some parameters in the example to configure unequal nss values iso of hardcoded ones?

> > 
> > FYI I've pushed your fix for bug 2606.
> 
> Great!
Comment 14 Matías Richart 2017-02-02 15:49:40 UTC
(In reply to sebastien.deronne from comment #13)
> (In reply to Matías Richart from comment #12)
> > (In reply to sebastien.deronne from comment #11)
> > > (In reply to Matías Richart from comment #10)
> > > > Created attachment 2774 [details]
> > > > Patch to test different SS at sender and receiver in ideal
> > > > 
> > > > A simple patch to try the new proposal.
> > > > We should extend the example to test all possible combinations.
> > > > 
> > > > My test: ./waf --run "ideal-wifi-manager-example --standard=802.11n-5GHz
> > > > --channelWidth=20 --shortGuard=0 --nss=4"
> > > 
> > > Thanks.
> > > I do not understand why you do those checks for VHT only?
> > 
> > Do you mean the NSS check?
> > In HT, we have a different mode indexes for each value of NSS. So, as we use
> > GetMcsSupported (station) we know the mode (and NSS) is supported by the
> > receiver.
> > 
> > In VHT, the same mode index can identify different NSSs, so I have to try
> > all possible NSSs and check if the receiver supports them.
> > 
> 
> Indeed, thanks.
> Can't we imagine to have some parameters in the example to configure unequal
> nss values iso of hardcoded ones?

Yes, sure. This was a quick workaround to test the proposal.
I'll work on that. I'm thinking also to include more parameters to have unequal channel widths and guard intervals.

> 
> > > 
> > > FYI I've pushed your fix for bug 2606.
> > 
> > Great!
Comment 15 sebastien.deronne 2017-02-02 16:13:43 UTC
(In reply to Matías Richart from comment #14)
> (In reply to sebastien.deronne from comment #13)
> > (In reply to Matías Richart from comment #12)
> > > (In reply to sebastien.deronne from comment #11)
> > > > (In reply to Matías Richart from comment #10)
> > > > > Created attachment 2774 [details]
> > > > > Patch to test different SS at sender and receiver in ideal
> > > > > 
> > > > > A simple patch to try the new proposal.
> > > > > We should extend the example to test all possible combinations.
> > > > > 
> > > > > My test: ./waf --run "ideal-wifi-manager-example --standard=802.11n-5GHz
> > > > > --channelWidth=20 --shortGuard=0 --nss=4"
> > > > 
> > > > Thanks.
> > > > I do not understand why you do those checks for VHT only?
> > > 
> > > Do you mean the NSS check?
> > > In HT, we have a different mode indexes for each value of NSS. So, as we use
> > > GetMcsSupported (station) we know the mode (and NSS) is supported by the
> > > receiver.
> > > 
> > > In VHT, the same mode index can identify different NSSs, so I have to try
> > > all possible NSSs and check if the receiver supports them.
> > > 
> > 
> > Indeed, thanks.
> > Can't we imagine to have some parameters in the example to configure unequal
> > nss values iso of hardcoded ones?
> 
> Yes, sure. This was a quick workaround to test the proposal.
> I'll work on that. I'm thinking also to include more parameters to have
> unequal channel widths and guard intervals.
> 
> > 
> > > > 
> > > > FYI I've pushed your fix for bug 2606.
> > > 
> > > Great!

Thanks for taking care of that.
Comment 16 sebastien.deronne 2017-02-05 07:55:04 UTC
To be delivered once ideal example is updated by Matias
Comment 17 Matías Richart 2017-02-06 14:27:49 UTC
(In reply to sebastien.deronne from comment #16)
> To be delivered once ideal example is updated by Matias

I uploaded a new patch with some improvements (in initialization there was missing the loop for NSS in VHT).

I also added a patch for ideal example which allows to have different standard, channel width, SGI and NSS in sender and receiver.

I did not yet test all possible combinations.
However, I noticed one discrepancy with previous version of the manager. When using ac and NSS=4 in both sides ideal selects a lower rate. 
As the new ideal code, tests all possible values for nss, it founds a rate with lower nss but with higher threshold and selects that. So, i don't know if this is a bug or the correct behavior of ideal.
Perhaps Tom can help with this.
Comment 18 Matías Richart 2017-02-06 14:28:16 UTC
Created attachment 2777 [details]
Fix bug and include checking for SS support at the receiver
Comment 19 Matías Richart 2017-02-06 14:28:43 UTC
Created attachment 2778 [details]
Patch to extend ideal example
Comment 20 sebastien.deronne 2017-02-06 14:46:02 UTC
(In reply to Matías Richart from comment #17)
> (In reply to sebastien.deronne from comment #16)
> > To be delivered once ideal example is updated by Matias
> 
> I uploaded a new patch with some improvements (in initialization there was
> missing the loop for NSS in VHT).
> 
> I also added a patch for ideal example which allows to have different
> standard, channel width, SGI and NSS in sender and receiver.
> 
> I did not yet test all possible combinations.
> However, I noticed one discrepancy with previous version of the manager.
> When using ac and NSS=4 in both sides ideal selects a lower rate. 
> As the new ideal code, tests all possible values for nss, it founds a rate
> with lower nss but with higher threshold and selects that. So, i don't know
> if this is a bug or the correct behavior of ideal.
> Perhaps Tom can help with this.

Thanks a lot.

Can we also adapt minstrel-ht example, or maybe merge them since they are quite similar?
Maybe it would also be good to add an option to be in infrastructure or adhoc?
I can do it this week if you miss time for this final improvement.

Could you detail a bit more the issue you encoutered?
Comment 21 Matías Richart 2017-02-06 15:16:40 UTC
(In reply to sebastien.deronne from comment #20)
> (In reply to Matías Richart from comment #17)
> > (In reply to sebastien.deronne from comment #16)
> > > To be delivered once ideal example is updated by Matias
> > 
> > I uploaded a new patch with some improvements (in initialization there was
> > missing the loop for NSS in VHT).
> > 
> > I also added a patch for ideal example which allows to have different
> > standard, channel width, SGI and NSS in sender and receiver.
> > 
> > I did not yet test all possible combinations.
> > However, I noticed one discrepancy with previous version of the manager.
> > When using ac and NSS=4 in both sides ideal selects a lower rate. 
> > As the new ideal code, tests all possible values for nss, it founds a rate
> > with lower nss but with higher threshold and selects that. So, i don't know
> > if this is a bug or the correct behavior of ideal.
> > Perhaps Tom can help with this.
> 
> Thanks a lot.
> 
> Can we also adapt minstrel-ht example, or maybe merge them since they are
> quite similar?
> Maybe it would also be good to add an option to be in infrastructure or
> adhoc?
> I can do it this week if you miss time for this final improvement.

I can look at it by the end of the week, not sooner :(

> 
> Could you detail a bit more the issue you encoutered?

With 802.11ac and nss=4 (same conf in both sides) I get lower rates. For example, with the new code the higher data rate selected is 260mbps while before was 312.

This is because with the old code only nss=4 was used and the rate selected was VhtMcs8 with nss=4.
Now, as ideal manager tests all possible values of nss, it founds that the "best" rate (the one with higher snr threshold) is VhtMcs9 with nss=3, which gets a lower data rate.
Comment 22 sebastien.deronne 2017-02-06 15:18:56 UTC
OK thanks, depending on my availabilities, I'll try to help as well.
Comment 23 sebastien.deronne 2017-02-08 17:20:41 UTC
(In reply to sebastien.deronne from comment #22)
> OK thanks, depending on my availabilities, I'll try to help as well.

I started working on the example.
If you have time, please focus on the VhtMcs8 with nss=4 case.
Comment 24 sebastien.deronne 2017-02-09 16:19:39 UTC
By the way, did you try running with a longer stepTime?

Example is almost ready.

What we might do is deliver the patch + example and open a new thread if bug is confirmed.
Comment 25 Matías Richart 2017-02-10 10:43:37 UTC
(In reply to sebastien.deronne from comment #24)
> By the way, did you try running with a longer stepTime?

I don't. Is it generating problems?

> 
> Example is almost ready.
> 
> What we might do is deliver the patch + example and open a new thread if bug
> is confirmed.

I agree.

Regarding the possible bug the difference is caused because the SNR threshold for the "same" rate with different NSS values is always the same.
 
In the example I tested what is happening is:
VhtMcs8 gets a SNR threshold of 600 (for nss: 1,2,3 and 4)
VhtMcs9 gets a SNR threshold of 900 (for nss: 1,2 and 3, nss=4 is invalid in this case)

So ideal manager selects Vhtmcs9 with nss=3.
Before the proposed patch, as we did not consider all possible values of NSS, ideal was choosing VhtMcs8 with nss=4 which provides a higher data rate than VhtMcs9, nss=3.

In summary, I think ideal is working as expected. What I do not know is if different NSSs should give different SNRs.
Comment 26 sebastien.deronne 2017-02-12 04:41:42 UTC
Created attachment 2783 [details]
example to replace ideal and minstrelht examples

I merged Ideal and MinstrelHt examples since both were quite duplicates. I took Matias's improvements to select different configuration on server and on client.
This also enables to test all managers and it is no longer limited to Ideal and MinstrelHt.
Comment 27 sebastien.deronne 2017-02-12 04:42:31 UTC
(In reply to Matías Richart from comment #25)
> (In reply to sebastien.deronne from comment #24)
> > By the way, did you try running with a longer stepTime?
> 
> I don't. Is it generating problems?
> 
> > 
> > Example is almost ready.
> > 
> > What we might do is deliver the patch + example and open a new thread if bug
> > is confirmed.
> 
> I agree.
> 
> Regarding the possible bug the difference is caused because the SNR
> threshold for the "same" rate with different NSS values is always the same.
>  
> In the example I tested what is happening is:
> VhtMcs8 gets a SNR threshold of 600 (for nss: 1,2,3 and 4)
> VhtMcs9 gets a SNR threshold of 900 (for nss: 1,2 and 3, nss=4 is invalid in
> this case)
> 
> So ideal manager selects Vhtmcs9 with nss=3.
> Before the proposed patch, as we did not consider all possible values of
> NSS, ideal was choosing VhtMcs8 with nss=4 which provides a higher data rate
> than VhtMcs9, nss=3.
> 
> In summary, I think ideal is working as expected. What I do not know is if
> different NSSs should give different SNRs.

Should ideal not search for the highest PHY rate iso the highest MCS?
Comment 28 Rediet 2017-02-13 07:04:10 UTC
(In reply to Matías Richart from comment #25)
> (In reply to sebastien.deronne from comment #24)
> > By the way, did you try running with a longer stepTime?
> 
> I don't. Is it generating problems?
> 
> > 
> > Example is almost ready.
> > 
> > What we might do is deliver the patch + example and open a new thread if bug
> > is confirmed.
> 
> I agree.
> 
> Regarding the possible bug the difference is caused because the SNR
> threshold for the "same" rate with different NSS values is always the same.
>  
> In the example I tested what is happening is:
> VhtMcs8 gets a SNR threshold of 600 (for nss: 1,2,3 and 4)
> VhtMcs9 gets a SNR threshold of 900 (for nss: 1,2 and 3, nss=4 is invalid in
> this case)
> 
> So ideal manager selects Vhtmcs9 with nss=3.
> Before the proposed patch, as we did not consider all possible values of
> NSS, ideal was choosing VhtMcs8 with nss=4 which provides a higher data rate
> than VhtMcs9, nss=3.
> 
> In summary, I think ideal is working as expected. What I do not know is if
> different NSSs should give different SNRs.

Hello Matías, Sébastien,
At the risk of beating a dead horse, here is a potential explanation: from a signal processing point of view, transmitting fewer NSSs over the same number of transmit antennas should result in greater diversity and thus greater (combined) SNR.
Cheers,
Rediet
Comment 29 Matías Richart 2017-02-13 10:18:14 UTC
(In reply to sebastien.deronne from comment #27)
> (In reply to Matías Richart from comment #25)
> > (In reply to sebastien.deronne from comment #24)
> > > By the way, did you try running with a longer stepTime?
> > 
> > I don't. Is it generating problems?
> > 
> > > 
> > > Example is almost ready.
> > > 
> > > What we might do is deliver the patch + example and open a new thread if bug
> > > is confirmed.
> > 
> > I agree.
> > 
> > Regarding the possible bug the difference is caused because the SNR
> > threshold for the "same" rate with different NSS values is always the same.
> >  
> > In the example I tested what is happening is:
> > VhtMcs8 gets a SNR threshold of 600 (for nss: 1,2,3 and 4)
> > VhtMcs9 gets a SNR threshold of 900 (for nss: 1,2 and 3, nss=4 is invalid in
> > this case)
> > 
> > So ideal manager selects Vhtmcs9 with nss=3.
> > Before the proposed patch, as we did not consider all possible values of
> > NSS, ideal was choosing VhtMcs8 with nss=4 which provides a higher data rate
> > than VhtMcs9, nss=3.
> > 
> > In summary, I think ideal is working as expected. What I do not know is if
> > different NSSs should give different SNRs.
> 
> Should ideal not search for the highest PHY rate iso the highest MCS?

From what I understand, ideal searches for the rate which gives higher SNR threshold (the rate which has higher minimum SNR for the given BER).
I think that the rationale behind this is that this would give the highest phy rate. But, as I mentioned, this is not the case when you add the NSS variable.
Perhaps Tom has a better view about this.
Comment 30 Tom Henderson 2017-02-13 11:39:21 UTC
(In reply to Matías Richart from comment #29)
> (In reply to sebastien.deronne from comment #27)
> > (In reply to Matías Richart from comment #25)
> > > (In reply to sebastien.deronne from comment #24)
> > > > By the way, did you try running with a longer stepTime?
> > > 
> > > I don't. Is it generating problems?
> > > 
> > > > 
> > > > Example is almost ready.
> > > > 
> > > > What we might do is deliver the patch + example and open a new thread if bug
> > > > is confirmed.
> > > 
> > > I agree.
> > > 
> > > Regarding the possible bug the difference is caused because the SNR
> > > threshold for the "same" rate with different NSS values is always the same.
> > >  
> > > In the example I tested what is happening is:
> > > VhtMcs8 gets a SNR threshold of 600 (for nss: 1,2,3 and 4)
> > > VhtMcs9 gets a SNR threshold of 900 (for nss: 1,2 and 3, nss=4 is invalid in
> > > this case)
> > > 
> > > So ideal manager selects Vhtmcs9 with nss=3.
> > > Before the proposed patch, as we did not consider all possible values of
> > > NSS, ideal was choosing VhtMcs8 with nss=4 which provides a higher data rate
> > > than VhtMcs9, nss=3.
> > > 
> > > In summary, I think ideal is working as expected. What I do not know is if
> > > different NSSs should give different SNRs.
> > 
> > Should ideal not search for the highest PHY rate iso the highest MCS?
> 
> From what I understand, ideal searches for the rate which gives higher SNR
> threshold (the rate which has higher minimum SNR for the given BER).
> I think that the rationale behind this is that this would give the highest
> phy rate. But, as I mentioned, this is not the case when you add the NSS
> variable.
> Perhaps Tom has a better view about this.

I agree that the current code seems to have been designed with this goal in mind, not taking into consideration NSS or channel bonding (which were not supported when Ideal was originally written).

I think that the logic may need to be rewritten to optimize maximum phy rate for the available SNR, or to perhaps approximate the logic of minstrel HT or other dynamic rate controls in terms of how they prioritize rates to search and select.
Comment 31 sebastien.deronne 2017-02-13 15:03:45 UTC
That is something we should indeed keep in mind. Nevertheless I think the scope of this bug has been addressed and I suggest to deliver code changes and close it.
Comment 32 sebastien.deronne 2017-02-14 02:25:37 UTC
Fix pushed in changeset 12686:503f88506643 and example pushed in changeset 12687:05d210a15753