-
Notifications
You must be signed in to change notification settings - Fork 5
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
Reproducibly set random number generators #528
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- pull in SOILWAT2 branch "feature_pcg_seeding" * now, a random number sequence can be exactly reproduced ("initstate" and "initseq"), see DrylandEcology/SOILWAT2#327 * see also DrylandEcology/SOILWAT2#326 * function `RandSeed()` has now two arguments "initstate" and "initseq" -> update STEPWAT2 calls to `RandSeed()` with new arguments - new `set_all_rngs()` to set each STEPWAT2 random number generator to produce sequences of random numbers that are reproducible (if user-provided "seed" is non-zero) and * unique among RNGs, iterations, years, and grid cells (most RNGs) * unique among RNGs, iterations, and years but identical among grid cells (weather generator RNG). * A user-provided "seed" of zero produces non-reproducible random number sequences which are non-coinciding among RNGs, iterations, and grid cells. - both non-gridded and gridded mode exactly reproduce weather among runs if seed != 0; weather is not reproduced among runs if seed == 0 - note: gridded mode reproduces weather almost but not exactly among cells (this is not the intended behavior and requires further investigation)
@kpalmqui I finally got around to implement the updates to the random number generators that we discussed a couple weeks ago! It appears to work ok except that grid cells do not exactly (but almost exactly) reproduce weather in gridded mode. |
- SOILWAT2 commit 947ae2f1b8a67119ff70762be5115d0ada60d15c updated "weathsetup.in" - changes are SOILWAT2-standalone but reading that file correctly is required for STEPWAT2
This includes - `SW_WTH_init_run()` now also initializes yesterday's weather values
The problem was that gridded mode used the last values in a year of one cell as first values of the next cell. This went mostly unnoticed but when the last day contains precipitation, then this affects the weather generator's behavior for the first day of the next cell. This is fixed by (i) SOILWAT2's `SW_WTH_init_run()` did not zero out yesterday's weather values; this was an issue only for STEPWAT2's gridded mode which does not deconstruct/construct each SOILWAT2 run (fixed with commit 8a0a754). (ii) STEPWAT2's `load_cell()` did not call `SW_CTL_init_run()` (and `SW_WTH_init_run()`) or equivalently to prevent the carry-over of values from one cell to the next (fixed with this commit). This now works as expected: i.e., * if seed != 0 (output is reproduced among runs) ** weather is exactly identical among runs and cells ** weather is different among years, iterations and seeds * if seed == 0 (output cannot be reproduced among runs) ** weather is different among cells, years, iterations and runs However and ideally, a grid cell should continue with the state that it ended at during the previous year (and not be zeroed out).
This now works as expected: i.e.,
Script to check expectations (requires to manually adjust seed during interactive use):
|
kpalmqui
approved these changes
Sep 14, 2022
dschlaep
added a commit
that referenced
this pull request
Oct 5, 2022
- SOILWAT2 branch "feature_read_weather" isolated the handling of daily weather data and moved it from within the simulation loop to the overall setup process - STEPWAT2 needs now to handle daily weather data itself; there are two basic options: i) follow SOILWAT2's new approach and generate daily weather for all years of a simulation run (for each grid cell and iteration); this would require that each grid cell stores and handles a local copy of `SW_Weather` ii) stick with the previous approach which generated daily weather for each year - this commit follows option (ii), i.e., generate daily weather for each year ** new `_sxw_generate_weather()` handles the generation of daily weather for the current year ** `Env_Generate()` now calls `_sxw_generate_weather()` before running SOILWAT2 for the current year ** non-gridded mode needed to set RNGs for each year (so that `markov_rng` gets updated with fresh values for each year) -> this commit satisfies expectations, i.e., (script based on #528 (comment)) - if seed != 0 (output is reproduced among runs) ** weather is exactly identical among runs and cells ** weather is different among years, iterations and seeds - if seed == 0 (output cannot be reproduced among runs) ** weather is different among cells, years, iterations and runs
dschlaep
added a commit
that referenced
this pull request
Jun 30, 2023
STEPWAT2 should meet reproducibility expectations as formulated with PR #528 (#528) that was merged into the main branch on Sep 18, 2022 with commit (7dce3c7) "Exactly reproduce random number sequences" This new bash script automatically runs gridded and nongridded example runs with STEPWAT2 using different seeds and uses 'diff' and 'Rscript' to check the following: * if seed != 0 (output is reproduced among runs) ** weather is exactly identical among runs and cells ** weather is different among years, iterations and seeds * if seed == 0 (output cannot be reproduced among runs) ** weather is different among cells, years, iterations and runs Note that 'master' and 'Seed_Dispersal' branches use different naming schemes of output files -> these need to be manually adjusted with variables `tag_gridded_biomass_cellk` (line 30) and `fname_gridded_biomass_meancell` (line 35) -- they are currently set to the 'Seed_Dispersal' scheme
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
RandNorm()
may not reproduce first random number SOILWAT2#326RandSeed()
has now two arguments "initstate" and "initseq"-> update STEPWAT2 calls to
RandSeed()
with new argumentsset_all_rngs()
to set each STEPWAT2 random number generator to produce sequences of random numbers that are reproducible (if user-provided "seed" is non-zero) and