Fixes AgentRandom uniform int

[Robadob]

Tested 600 ints (from the same thread) in the range [0, 5]

Got this histogram, which seems as you would expect.

0: 118
1: 86
2: 89
3: 109
4: 90
5: 110

curand rng returns in the range (0, 1], so the math is a bit arkwards.
Furthermore, as we return an inclusive range, we must add an additional 1 to the range (max-min).
Perhaps there's a slight argument for the upper bound to be exclusive to simplify the maths.

[ptheywood]

Probably worth aiming to be consistent with what users would expect in terms of inclusivity.

std::uniform_int_distribution is a <= i <= b.
Pythons random.randint(a, b) is a <= N <= b (although randrange(start, stop) also exists as start <= N < stop

In terms of floating point:

• std::uniform_real_distribution is a <= x < b (which is apparently the "rectangular distribution" of [a,b)
• Python random.random() is [0.0, 1.0)

So curand seems to differ from what standard libraries suggest. Would it not be better to modify AgentRandom::uniform() so that it returns [0.0, 1.0) instead of curands (0.0, 1.0], then the other methods would be cleaner and curand would be more like the rest of the world? Might be a key point when porting from flamegpu 1 though.

[Robadob]

re: floating point.
Trivial change, I just err on the side of 'that's more ops'.
Could be an argument to bench the variations to see how costly they are. Probably not very unless doing it inside a loop.

[ptheywood]

I would assume that the cost of an extra floating point subtraction is going to be negligible compared to the cost of actually generating a random number, but yeah benching it is never a bad idea.

Could also expand the testing of RNG (#330) while i'm blocking changes to CUDASimulation.

[Robadob]

I'm not really sure how #330 should look. I certainly don't think it will fit inside the regular test suite.
Random is random, one trusts it does what it's documented to do (we're not the ones implementing the underlying random engine).

[ptheywood]

No, but we are documenting the inclusivity of the ranges for float and int variants, so atleast verifying that would be good.
The random integer generation is also done by us (from the curand float) so it's probably worth making sure that distribution is approximately correct (as a common mistake with doing that (in C atleast) is to have too few at the upper bound due to rounding)

[Robadob]

The int generation works now, small histogram suggests its got no huge bias, and I can't see it being changed further (besides lopping off the inclusive upper bound), so that doesn't seem like an issue.

[ptheywood]

Int generation seems to be inclusive most places so i'd keep it inclusive.

Ranges should certainly be tested though (as changing what uniform returns would potentailyl lead to out of bounds etc) and worth having a test.

[Robadob]

I want to make some more changes to this before it's merged (mostly to how double uniform is calculated)

[Robadob]

Actually can't do what I thought I could, so won't bother.

[Robadob]

Windows/Debug/SEATBELTS=ON Python tests pass (I wanted to check RTC would build the updated header)

===================================== 648 passed, 11 skipped in 331.66s (0:05:31) =====================================

[ptheywood]

Changes look OK at a glance + your testing mentioned on slack sounds OK. Worth getting merged in during 2.0.0-rcX given its potentially a significant change for users.

Still needs more thorough testing eventually so we'll keep #330 open.