josdejong / mathjs

An extensive math library for JavaScript and Node.js
https://mathjs.org
Apache License 2.0
14.4k stars 1.24k forks source link

randomSeed in `math.config` does not trigger an update when the same value is used #1994

Open kcf-jackson opened 4 years ago

kcf-jackson commented 4 years ago

The randomSeed option of the math.config call does not seem to trigger an update when the same value is used.

// Checked with the browser
math.version
> "7.5.1"
math.config({randomSeed: 100}); math.random(1);
> 0.6781078499227314
math.config({randomSeed: 100}); math.random(1);
> 0.35155301939509676
// Reset
math.config({randomSeed: null}); math.random(1);
> 0.7179467920588589
math.config({randomSeed: 100}); math.random(1);
> 0.6781078499227314
// Checked with Node.js
> const {create, all, version} = require("mathjs")
undefined
> version
'7.5.1'
> const math = create(all, {})
undefined
> math.config({randomSeed:100}); math.random();
0.6781078499227314
> math.config({randomSeed:100}); math.random();
0.35155301939509676
// Reset
> math.config({randomSeed:null}); math.random();
0.749174817891811
> math.config({randomSeed:100}); math.random();
0.6781078499227314

The issue has been addressed in issue 799, but the problem seems to come back at #1319 with the addition of curr.randomSeed !== prev.randomSeed in random.js, randomInt.js and pickRandom.js.

The check is needed because "randomSeed" in math.config remains a constant, so when the random number generator (RNG) is created on the fly, it has to avoid starting the RNG from scratch, or the generated numbers will also be constants.

~A workaround is perhaps to use some kind of system time as the default seed when the RNG is initiated, but the current choice may have been made to ensure cross-platform compatibility. So I am not sure if this is a feature or a defect. I wonder if #1955 would fix it automatically.~ Edit: On closer inspection, it seems both seed-random and seedrandom would auto-seed when no argument is provided, so the above is relevant only when "randomSeed" is set to something not equal to null. I apologise if I did not explain the issue clearly. In short, I think a clarification is needed on what one should expect when randomSeed is set in math.config. Does it mean

I am confused because the current code seems to do a bit of both. It fails the first one, because when a seed is set, the first and second runs of random gives different results, and it fails the second one, because if the same seed is provided twice in a row, it does not give the same result.

While on the topic, may I also ask if there is a way to set the seed if I import only the random function from mathjs? I expect negative but would be happy to be corrected. Thanks!

josdejong commented 4 years ago

That is a good point, thanks for bringing this up.

The randomSeed option is a bit of a special case: for all config, you want to be lazy and only actually do work when the config value actually changes. In case of randomSeed this is different. Currently, the on('config', ...) event triggers when "something" changes in the config. The if (curr.randomSeed !== prev.randomSeed) there to prevent setting a seed again when doing an unrelated config change like math.config('number': 'BigNumber'). Currently, there is no way to know at that point whether a value for randomSeed was actually passed by the user.

What will work is create a new mathjs instance:

math.config({randomSeed: 100}); math.random(1);
> 0.6781078499227314
math = math.create({randomSeed: 100}); math.random(1);
> 0.6781078499227314

I'm not yet sure how we can solve this properly.

While on the topic, may I also ask if there is a way to set the seed if I import only the random function from mathjs? I expect negative but would be happy to be corrected. Thanks!

I'm not entirely sure what you mean, but like explained in the example above, at the moment you create a new mathjs instance the provided seed is applied.