Re: [Discuss-gnuradio] fastnoise_source is not thread-safe

[CEL]

Hello Markus,

just to add to the things said:
The (non-fast) noise source in GR has experienced a bit of bugfixing
(but more on a mathematical level: turns out that the built-in random
function GNU Radio used wasn't all that random if one looked for about
10^8 samples)...

I'm currently reading the fastnoise_source block's code, I'm really not
all that sure it's *fast*. I mean, neither `rand()` nor `indirectly
addressing a RAM buffer that's larger than a CPU cache` are really an
indication for speed. The reason why it's still faster than the noise
source is that the distribution shaping happens offline (which means
that probably, our distribution shaping is suboptimal; then again, the
method used in the generation of normally distributed variables can be
found all over the internet, probably because it's the method in
/Numerical Recipes/, and it's a cool method, anyway!).

That having been said, maybe, instead of just fixing fastnoise source
(which we must do, I agree!) only, we might simply want to look into
replacing it by something faster yet threadsafe. I'll whip something up
for benchmarking purposes; XOROSHIRO128+ has an excellent reputation
with respect to both speed and statistical properties, and I'd be
surprised if we couldn't use it to generate both random floats as well
as random integers. Problem, as usual, is that a good random number
generator does not make a good source of (pseudo)continuous or even
just discrete randomness of any given distribution. However, I'm
willing to test a few reference distributions (mainly, Numpy's, because
they're easy to use in tests and hopefully well-tested themselves) for
distributional similarity¹ to the expected distributions, and some
auto- and crosscorrelation tests.

So, hm, how do we do this?

Anyway: Seeing you post on the mailing list lately I'd say it really
can't hurt letting you sign the FSF CRA; I think you'd greatly benefit
of having the possibility to quickly upstream a fix, even if it's
complex. For smaller fixes, a CRA isn't always strictly necessary, but
your friendly maintainer knows more :)

Best regards,
Marcus



¹ seriously, I'll have to restrict myself to a Kolmogorov-Smirnov test,
and maybe Kullback-Leibler divergence considerations. That's all I can
honestly claim to understand well enough to make qualified statements.

On Fri, 2018-01-26 at 00:45 +0100, Markus Wirsing wrote:
> Hello,
> 
> I noticed that the fastnoise_source block is not thread safe, as it
> calls rand(), which keeps global state.
> This makes it non-reproducable between multiple runs once more than one
> such source is used.
> 
> I also opened an issue about this on github:
> https://github.com/gnuradio/gnuradio/issues/1542
> However, there were no reactions to it.
> 
> I'm considering to write a fix for it when I find the time.
> However, I'm not sure about the requirements that need to be met that it
> can later be included in gnuradio.
> 
> Would this already require me to have a CRA?
> I'm also wondering whether C++11 is allowed in gnuradio.
> I guess it is not as I found this message on the mailing list:
> https://lists.gnu.org/archive/html/discuss-gnuradio/2014-06/msg00014.html
> It might be worth mentioning that on the coding guide in the wiki though:
> https://wiki.gnuradio.org/index.php/Coding_guide_impl
> It also might be worth considering to change that rule.
> C++11 is pretty well supported by now. And it does add some nice features.
> 
> I think the easiest fix would be to use random::ran_int() in place of
> rand() or lrand48(). That way it would also be possible to get rid of
> the (slightly) biased method of using a linear congruence to reduce the
> range.
> 
> However, while looking at the code in random.cc
> https://github.com/gnuradio/gnuradio/blob/master/gnuradio-runtime/lib/math/random.cc
> I noticed that it keeps raw pointers to the different random generators.
> I don't see why this is done. If I don't misunderstand anything, it
> would work equally well to have the objects as members in the class.
> With the added benefit of having them correctly deconstructed in the
> case of exceptions. Even though that may not be of much relevance in
> this case.
> But it at least would make sense to use unique pointers instead of raw
> pointers.
> So if I find the time to create a fix for the other problem, would some
> cleanup in this place be appreciated too?
> 
> I have no experience so far with submitting code to open source projects.
> These issues seem to be a good starting point. However, before investing
> time, I want to make sure my work can actually be used.
> 
> Appreciating any feedback on the proposed changes and on the necessary
> formalities,
> Markus
> 
> _______________________________________________
> Discuss-gnuradio mailing list
> address@hidden
> https://lists.gnu.org/mailman/listinfo/discuss-gnuradio

smime.p7s
Description: S/MIME cryptographic signature