-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Integrate stateless random number generators into stochastic tools samplers. #32248
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
base: next
Are you sure you want to change the base?
Conversation
… stateless shuffle - Replace stateful RNG usage in stochastic_tools samplers with stateless indexing, including PMCMC-based and active learning samplers - Add stateless generator advance helper and LHS stateless shuffle with execute-time advance to preserve repeatable sampling Ref idaholab#32194
|
Job Documentation, step Docs: sync website on 5e8e17c wanted to post the following: View the site here This comment will be updated on new commits. |
|
Job Coverage, step Generate coverage on 3c64d5f wanted to post the following: Framework coverage
Modules coverageStochastic tools
Full coverage reportsReports
This comment will be updated on new commits. |
||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Job Coverage, step Verify coverage on 8693da3 wanted to post the following: The following coverage requirement(s) failed:
|
1 similar comment
|
Job Coverage, step Verify coverage on 8693da3 wanted to post the following: The following coverage requirement(s) failed:
|
zachmprince
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a great first start! This definitely doesn't close the issue yet, but I doubt it would in a single PR.
The biggest thing I'd like to see from this PR is the removal of sampleSetUp and sampleTearDown methods. This will pave the way to replacing getNextLocalRow logic throughout the module.
| std::size_t rn_ind = 0; | ||
| for (std::size_t i = n_global - 1; i > 0; --i) | ||
| { | ||
| const auto j = getRandlStateless(rn_ind++, 0, i, seed_index); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you look at the implementation on how the stateless RNG draws ranged integers, the memory footprint will blow up with this call. Basically, it will eventually create n_global - 1 objects. As such, I would recommend adding a new method in MooseRandomStateless specifically for grabbing indices for shuffling. I'm thinking the signature would look like this:
std::size_t randlShuffle(std::size_t n, std::size_t size)Which will effectively be the same as calling:
randl(n, 0, size - n - 1)but it will create a new generator object based on size instead of size - n - 1.
| @@ -12,6 +12,7 @@ | |||
| #include "Sampler.h" | |||
| #include "TransientInterface.h" | |||
| #include "Distribution.h" | |||
| #include <cstddef> | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these includes necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope, my editor inserted them. Will remove.
| const auto shuffle_count = n_rows > 0 ? n_rows - 1 : 0; | ||
| for (const auto col : make_range(n_cols)) | ||
| { | ||
| advanceStatelessGenerator(col, n_rows); | ||
| if (shuffle_count > 0) | ||
| advanceStatelessGenerator(col + n_cols, shuffle_count); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any way we can rely on the advancement step in Sampler::execute instead of having a specialized one here? Perhaps we could add a virtual protected member in Sampler to get the advance count for a particular seed index. I'd like to see as little customization of these execute* routines as possible.
- Drop sampleSetUp/sampleTearDown from the Sampler base and its call sites. - Move sampler preparation into explicit update/build helpers called from compute paths. - Add mode-aware computeSampleMatrix/Local/Row overrides for LHS and PSS. - Keep stateless RNG usage consistent and add small inline/doc updates. Ref. idaholab#32194
- Add MooseRandomStateless::randlShuffle and Sampler helper to avoid per-range generator growth during shuffles. - Update LatinHypercubeSampler to use the shuffle RNG path. - Remove sampleSetUp/sampleTearDown hooks; move sampler preparation into explicit update/build paths. - Update latin_hypercube gold CSVs because the new shuffle RNG changes the deterministic sequence.
- add stateless advance hooks in Sampler and wire execution to use per-seed advance counts - advance shuffle generators in MooseRandomStateless::advance to keep stateless shuffles aligned - adjust LatinHypercubeSampler to use stateless shuffle advances and integrate with execute flow - regold latin_hypercube_out_data_0001.csv for the new deterministic sequence Ref. idaholab#32194
|
Job Precheck, step Clang format on 7cc471e wanted to post the following: Your code requires style changes. A patch was auto generated and copied here
Alternatively, with your repository up to date and in the top level of your repository:
|
|
Job Test, step Results summary on 5e8e17c wanted to post the following: Framework test summaryCompared against e4407b3 in job civet.inl.gov/job/3542080. No change Modules test summaryCompared against e4407b3 in job civet.inl.gov/job/3542080. No added testsRun time changes
|
Ref #32194.