Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Random seed not respected when specified in configuration file that is loaded automatically given a library path and configuration file name #141

Closed
hrnorton opened this issue Mar 12, 2024 · 3 comments · Fixed by #153
Labels
bug Something isn't working complete-awaiting-release

Comments

@hrnorton
Copy link

hrnorton commented Mar 12, 2024

I seem to have found a small bug in which the random seed I set in my configuration file is not being used in the simulation.

I created my simulation using Simulation() rather than the Simulation.from_config(). Unlike when using the from_config function, when using the default class initialization my random seed loads correctly as part of the Simulation.config attribute but is not applied to the Simulation.random_seed attribute directly. When Simulation._setup_simulation() calls, it passes only the random_seed attribute on the simulation object, not the version from the config attribute, when it instantiates the WombatEnvironment class. As a result, the environment detects no random seed.

Apologies if I have misunderstood something, but it seems like this is probably not the desired behaviour. Everything works fine if I pass the random seed directly as a parameter into the Simulation() function, or if I load using from_config(), so it seems to just be this one case which has the problem.

I am using WOMBAT v0.9.3.

@RHammond2
Copy link
Contributor

Thanks for catching this, @hrnorton! This seems to be a design inconsistency issue, so I should be able to get a quick fix for this into the develop branch. Would the desired behavior be to use the configuration's random seed if one isn't provided to the Simulation() call, otherwise have Simulation's override the configuration input?

@RHammond2
Copy link
Contributor

@hrnorton I've just opened PR #142, please let me know if this resolves your issue, or if there's something different you're needing for this fix.

@hrnorton
Copy link
Author

That works, thanks!

@RHammond2 RHammond2 mentioned this issue Jul 1, 2024
@RHammond2 RHammond2 linked a pull request Jul 1, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working complete-awaiting-release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants