Add seed parameter to UseCase #128
Conversation
|
I think this feature does not need to be tested, but we could now write some tests thanks to it. The first one I would have in mind would be to save outputs from input_file_i.py computed with given seed (maybe not 30 runs but just 1) and then automatically make sure those results are the same (this would mean save the outputs once as csv file for comparison) |
|
This looks like a sensible way to implement the Could you perhaps show a couple of example outputs with the random seed? |
|
Hi Francesco, It generates this output of three example use cases (based on input_file_1) with and without fixed random seed. |
There was a problem hiding this comment.
The changes proposed look good, and the provided example confirms that they work as expected.
The only doubt I have is whether we want to keep the random_seed_example.py as part of the merged code or not. If we do, we may want to add some documentation that explains why it is there.
Additionally: check out the guidelines for contributing; I believe you are missing the addition of your name to the AUTHORS list.
|
Good point, I just added myself to the AUTHORS list. It's just a small feature so I believe it would be fine without demonstration example. Alternatively I could create a notebook based on |
|
Hi, sorry for the delayed response. I think that it wouldn't be a bad idea to have an example of how to employ the random seed, given that we think this could be useful for a few cases. If you can build a notebook with short documentation, I'd be happy to have it included in the docs. |
|
Hi, sorry for my delayed reaction. I just added a notebook demonstrating the functionality. Does that look good? |
| @@ -100,6 +105,10 @@ def __init__( | |||
| if self.date_start is not None and self.date_end is not None: | |||
| self.initialize() | |||
|
|
|||
| # Set global random seed if it is specified | |||
| if self.random_seed: | |||
There was a problem hiding this comment.
| if self.random_seed: | |
| if self.random_seed is not None: |
I prefer the defensive coding :)
There was a problem hiding this comment.
@JW-Kraft - could you rebase this branch onto the latest joss-paper and force push it?
JW-Kraft
force-pushed
the
review/set-random-seed
branch
from
d615bc3
to
87fc130
Compare
Added the ability to provide a seed parameter for a UseCase as discussed in #127.
Manual testing with the existing input files gives reproducible results as expected.
Writing unit tests for this feature is tricks though, since pytest resets the random generator before each test to the same state (seed). This is exactly what this new feature is doing. Testing for it with pytest is therefore not useful: The test will not fail even if there is something wrong with setting the seed when initializing a UseCase.