<div><div dir="auto">Hi Fred</div></div><div dir="auto"><br></div><div dir="auto">There are two RNGs involved:</div><div dir="auto">1. C++ STL’s mt19937 seeded with 256 bits of entropy; a developer wanted to conserve OS entropy source, so that it did not use more entropy.</div><div dir="auto">2. OpenSSL’s RAND_bytes function.</div><div dir="auto"><br></div><div dir="auto">The original statistics method is a K-S test for goodness of fit. The implementation checks lower 5 bits of the random number over 35 samples.</div><div dir="auto">The test rewritten in attempted fix D is also K-S test. It checks upper 8 bits of the random number over 50 samples.</div><div dir="auto"><br></div><div dir="auto">My vote for short term would be E: revert it to soft failure, so that it does not block other commits; but the warning is kept as a reminder of this problem.</div><div dir="auto">For longer term, I could prefer F or G over D, but this needs someone that actually understands statistics. Developer who wrote D is a beginner in statistics.</div><div dir="auto"><br></div><div><div dir="auto">Yours, Junxiao</div><br><div class="gmail_quote"><div dir="ltr">On Fri, Jan 18, 2019 at 18:11 Fred Douglis <<a href="mailto:fdouglis@perspectalabs.com">fdouglis@perspectalabs.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div>
<p>Can I vote "none of the above"?</p>
<p>I think checking the RNG was a good idea, and simply removing the
test sounds bad. If in fact it's not really conforming to the
expected distribution, this is a problem. You haven't really said
much about the statistical method that declared it was failing,
other than that you could repeat the test a few times and
dramatically bring down the failure rate. If you trust the test,
it sounds like the RNG is in fact broken, but maybe it's more a
bad test than a bad RNG. If the test is good, then it seems like
you missed opinion F, which is to keep it as a hard failure and <b>force
a fix or change of libraries</b>. If the test is bad, then
opinion G is, <b>find a better test</b>. <br>
</p>
<p>Not that my opinion counts much, I'm new to the list, and so far
simply a lurker. <br>
</p>
<p>Fred<br>
</p></div><div>
<div class="m_-4037743747416091094moz-cite-prefix">On 1/18/2019 6:00 PM, Junxiao Shi
wrote:<br>
</div>
</div><div><blockquote type="cite"></blockquote></div><div><blockquote type="cite">
<div dir="auto">Dear folks</div>
<div dir="auto"><br>
</div>
<div dir="auto">There is an urgent developer disagreement during
code review related to issue 4808. It seems that this could not
wait until the next NFD call, so I’ll explain the facts here.</div>
<div dir="auto"><br>
</div>
<div dir="auto">The technical problem: ndn-cxx has a random number
generator implemented by calling into third party libraries.
There was a unit test using statistics method to check that the
generated random number conforms to a uniform distribution.
Given it’s a randomized test, the unit test fails “softly”: it
creates a warning when fails, not an error.</div>
<div dir="auto">Recent changes: developer A made a commit changing
the soft failure to hard failure. As a result, many Jenkins
builds are failing. An independent test indicates the failure
rate is 14% or more.</div>
<div dir="auto"><br>
</div>
<div dir="auto">Attempted fix A: delete the unit test outright
because it’s “semi-broken”. Afterwards, there would be no unit
test to check the numbers are random.</div>
<div dir="auto">Opinion B: “testing random number generator is not
ndn-cxx’s business”, so the unit test can be deleted.</div>
<div dir="auto">Opinion C: every line of code requires at least
one failing test. Therefore, without any unit test on the random
number generator, one could just make “return 0;” the random
number generator.</div>
<div dir="auto">Attempted fix D: rewrite the “goodness of fit”
unit test, loosening numerical requirements. Execute the test
five times, and declare a hard failure only if the test fails
three or more times out of five. The failure rate of this method
is 0.01%.</div>
<div dir="auto">Opinion E: revert to soft failure.</div>
<div dir="auto"><br>
</div>
<div dir="auto">I am one of the parties involved but I tried to
summerize the facts. I hope everyone (including myself) can calm
down and make a decision at next NFD call, and don’t rush with
merging one of the changes. Please keep all discussions on the
mailing list by using reply-all only.</div>
<div dir="auto"><br>
</div>
<div dir="auto">Yours, Junxiao</div></blockquote></div><div>
</div>
</blockquote></div></div>