activitysim icon indicating copy to clipboard operation
activitysim copied to clipboard

Fixed random seed generator to generate unsigned integers instead of …

Open JoeJimFlood opened this issue 2 years ago • 4 comments

The value of _MAX_SEED in random.py is currently set to the maximum 32-bit unsigned integer, but the default data type for np.random.RandomState().randint() is at 32-bit signed integer. This was causing an error when the default random seed was set to be None and triggering the random seed generation.

To test this, I ran the SANDAG 1-zone examples twice and the 3-zone example once, all of which were set to use random seeds. All runs completed successfully. I compared the mode share from the two 1-zone runs. They weren't exactly the same but at most were within 0.3%, which is what I would expect if the random seed generator worked correctly.

JoeJimFlood avatar Aug 03 '22 23:08 JoeJimFlood

Coverage Status

Coverage decreased (-1.4%) to 55.068% when pulling 90f073d83946034195a92c5eed45135e05d16f8f on JoeJimFlood:fix_gen_random_seed into 3df695bd2bf921aa46d296a09889b68087b8c911 on ActivitySim:master.

coveralls avatar Aug 03 '22 23:08 coveralls

@JoeJimFlood do you want to take a stab at writing a test to run in our continuous integration testing, that confirms this change works as expected? I envision one easy way would be setting the seed to None on to an existing test example and running to check that the final results to NOT exactly match the "expected" results with a zero seed.

jpn-- avatar Aug 29 '22 14:08 jpn--

@jpn-- I've never done that before but I'd love to try. Would I just need to edit the settings.yaml file in the test subdirectory of one of the examples? For example in this file?

JoeJimFlood avatar Aug 29 '22 18:08 JoeJimFlood

@jpn-- I had opened this pull request before Version 1.1 was released. Would it be better if I closed it and created a new one with the same changes applied to the v1.1 code?

JoeJimFlood avatar Sep 12 '22 15:09 JoeJimFlood

I added a test of the random number generator to the tests in actions. To test that I ran it using a version of the software that has that test but not the random generator bug fix, meaning that the random seed generator test should fail due to the seed always being zero. The results of that are here, where the random seed test failed and all the others passed. I guess that would mean... task_failed_successfully

JoeJimFlood avatar Nov 08 '22 17:11 JoeJimFlood

Looks good to me, thanks Joe!

dhensle avatar Dec 03 '22 00:12 dhensle