Arduino icon indicating copy to clipboard operation
Arduino copied to clipboard

randomSeed: Bad/redundant "if"

Open igendel opened this issue 9 years ago • 6 comments

Hi,

In function randomSeed (WMath.cpp, line 30) there's the code

  if (seed != 0) {
    srandom(seed);
  }

Since "srandom" handles the value 0 itself (see for example its implementation in random.c of AVR-Libc*), it seems there's no need for that "if". Furthermore, it is undocumented and may break functionality - for instance, if a user wants to produce two identical pseudo-random sequences using the seed 0.

  • https://github.com/vancegroup-mirrors/avr-libc/blob/master/avr-libc/libc/stdlib/random.c

If that "if" can be removed, it will save a byte or two.

See discussion about this here (not all of it directly relevant): https://forum.arduino.cc/t/randomseed-why-is-0-ignored/337424

Thanks,

igendel avatar Sep 28 '15 23:09 igendel

For srand(seed) - resp. randomSeed() - , perhaps 0 is handled seperately to avoid division by zero, but often this following is common practice to PRNGs:

seed== 0 // 0: a "real" randomized random seed based on system clock() seed==-1 // -1: restore last random series seed== either int value // start PRNG sequence at a pre-defined start value

Note that a PRNG initialized by a certain fixed value will always generate identical pseudo-random series to infinity, which can be used for code testing purposes - but which will not make much sense for common PRNG use!

As Arduino boards don't provide a system clock - and srand(0) or randomSeed(0) are not well-documented by Arduino reference at all - I'm using my own "randomized rand() initialization" by

srand ( (unsigned int)( millis() + AnalogRead(A0) ) );  // as a substitute to srand(0)

_(moreover, the Arduino API function names are not C compliant - rand(void) srand(int seed) had to be used instead, and RAND_MAX should be well-defined!)_ https://forum.arduino.cc/t/randomseed-why-is-0-ignored/337424/14

shiftleftplusone avatar Sep 29 '15 09:09 shiftleftplusone

Always nice to read some Vogon poetry ;-)

For the time being, let's separate here the general discussion on PRNGs from the existing Arduino implementation. Given the current state, where randomSeed is essentially a wrapper to AVR-Libc's srandom, the if in the function seems erroneous. Can we do without it?

igendel avatar Sep 29 '15 11:09 igendel

IMO we actually needed to have srand(0) for "randomized" seeding and srand(-1) to restore the last random series These C functionalities seems to be missing yet.

shiftleftplusone avatar Sep 29 '15 11:09 shiftleftplusone

@igendel, agreed, I cannot see any reason for treating "0" specially. The libc srandom function does not do it, and the randomSeed documentation does not indicate 0 is special either. Since libc says the default seed is '1', the current code means that randomSeed(0), randomSeed(1) and not calling randomSeed() now all result in the same seed, which seems unintended.

@VogonJeltz, you talk about rand() / srand(), which seems to be a different set of functions than the random() / srandom() used by Arduino (though some answers to this post suggest that on Linux, they both access the same random generator, but random() might be better). However, none of these functions actually seem to support 0 and -1 as a special value, AFAICS (Looking at the libc random and rand manpages). Do you have a link to any documentation that describes how these "C functionalities" should work? Though I don't think we would actually need special handling like that - on the Arduino you're in control, so you can pick the random seed (also note that an Arduino has no reliable source of randomness, so implementing automatic randomized seeding seems to be impossible).

matthijskooijman avatar Sep 29 '15 12:09 matthijskooijman

yes, I only know the common cstdlib C commands rand and srand. But about the (0) I stand corrected, I muddled it up - it was time(0), not just (0), sorry. for "randomized" seeds I take

srand ( (unsigned int)( millis() + AnalogRead(A0) ) );

About -1 I'm searching and will report... IIRC I once used it ...

shiftleftplusone avatar Sep 29 '15 13:09 shiftleftplusone

Proposal from #575: #define randomSeed(seed) srandom(seed)

ace-dent avatar Jun 17 '21 14:06 ace-dent