Bug 2589

Summary:	Binary search in LteMiErrorModel::GetTbDecodificationStats is incorrect
Product:	ns-3	Reporter:	Alexander Krotov <krotov>
Component:	lte	Assignee:	Biljana Bojović <bbojovic>
Status:	PATCH PENDING ---
Severity:	normal	CC:	mmiozzo, ns-bugs, tomh
Priority:	P3
Version:	ns-3-dev
Hardware:	All
OS:	All
Attachments:	Patch 1/2 Patch 1/2 Patch 2/2 (actual fix, apply on top of the first)

Description Alexander Krotov 2016-12-15 15:58:00 UTC

Created attachment 2702 [details]
Patch 1/2

Binary search at http://code.nsnam.org/ns-3-dev/file/f883d71c975e/src/lte/model/lte-mi-error-model.cc#l613 is incorrect. It does not give the intended result. At least the result is not the same as what commented code gives: http://code.nsnam.org/ns-3-dev/file/f883d71c975e/src/lte/model/lte-mi-error-model.cc#l604

I split the fix in two patches. First one uncomments the simple but slow version of code and moves it inside "#ifdef NS3_BUILD_PROFILE_DEBUG" to show that it indeed results in assertion failures during tests.

The second patch replaces the algorithm with the standard std::lower_bound. It follows in the next message.

Comment 1 Alexander Krotov 2016-12-15 15:58:46 UTC

Created attachment 2703 [details]
Patch 1/2

Reuploaded first patch as patch.

Comment 2 Alexander Krotov 2016-12-15 16:04:11 UTC

Created attachment 2704 [details]
Patch 2/2 (actual fix, apply on top of the first)

Here I wrapped cbSizeTable into std::array to use std::lower_bound. According to tests (no assertion failures) the code corresponds to the simple version of code.

I think we never discussed limits of C++11 usage in NS-3 code since switch, but I decided that using "auto" for iterator and lambda for a one-line comparison function is sane in this case.