Migrate the OAT SA generator to in-memory sampling - #4057
Conversation
…data generate_OAT_SA_design now samples in memory via load_pft_posteriors and get_parameter_samples, requesting SA quantiles without an ensemble draw, and returns list(X, samples) like the ensemble generator. Its sa_samples argument becomes samples, taking the full bundle so both generators are called the same way. .prepare_input_designs threads the resolved bundle into it, and warns that internal design generation is deprecated and passing input_design will become required. Add tests for the in-memory sampling, the pass-in path, the empty sa.samples guard, and the deprecation warning.
|
|
||
| for (input_type in input_types) { | ||
| if (input_type == "param") { | ||
| # sequential indices map to SA run order |
There was a problem hiding this comment.
There are a number of places, like this one, where the existing code documentation was useful and has been removed. Rather than me flagging dozens of places, I'd recommend looking at the PR diff on GH to see where comments/documentation can be restored
There was a problem hiding this comment.
followed up on this properly rather than spot-checking. counting the removed comment lines across the stack: 45 in the OAT generator, 30 in the ensemble generator, 6 in runModule and 4 across the two test files.
the OAT ones are restored. going through the rest, most were either describing the old get.parameter.samples and samples.Rdata behaviour, which would now be wrong, or were reworded for the X to design_matrix rename. one was a real loss: the "in practice it" list explaining how each PFT's posterior gets resolved, which i'd folded into prose. that's back as the original list in #4060, since it's still accurate.
if anything else reads thinner than before, point at it and i'll put it back.
|
fair catch, i stripped more than i should have. restored in #4060 as you suggested, since it builds on this one: the settings requirements list, the OAT design logic explanation, the worked example, and the inline comments on how the run count is derived and why every non-param column stays at 1. going back through both diffs for anything else i trimmed too hard. two bits i left out rather than restored: the note on internal dependencies describing what get.parameter.samples does, and settings$outdir listed as the output directory for samples.Rdata. the function no longer calls that wrapper or touches that file, so both would now be wrong. on the demo restructure you asked for, demo 1 going to a small ensemble with the visualizations updated is independent, so i'm doing that next in #4063. demo 2 becoming SA-only has to wait for the routing: right now whatever you pass as input_design lands in designs$ensemble and the SA config-writing branch only fires off designs$sensitivity, so handing in an OAT design would put it in the wrong slot. that's really the two-separate-calls change from slack, so i'll do it as its own PR and bring demo 2 with it rather than half-doing it here. |
2 participants
Week 6 of the modularity work, and the OAT half of the generator migration started in #4042. Stacked on #4054, so the diff will shrink once that merges — the OAT changes are the last commit.
what changed
generate_OAT_SA_design now samples in memory through load_pft_posteriors and get_parameter_samples, instead of calling the deprecated get.parameter.samples wrapper and reading sa.samples back out of samples.Rdata. It asks for the SA quantiles with do_ensemble = FALSE, since an SA needs the quantile-based samples rather than an ensemble draw.
Its sa_samples argument becomes samples and takes the full parameter bundle, matching generate_joint_ensemble_design. Both generators now have the same signature and both return list(X, samples), so .prepare_input_designs calls them the same way and threads one resolved bundle into either. The design-building logic itself is unchanged: param stays sequential, every other column stays at 1.
An empty sa.samples now errors instead of producing a design with no traits in it.
Also folds in the deprecation warning @mdietze asked for on #4042: .prepare_input_designs warns that internal design generation is going away and that passing input_design will become required. It only fires when a design is actually being auto-generated, so callers passing their own design don't see it.
why now
Per @mdietze on Slack, the OAT generator had to move or it would stop working with the new design, since the generators are the last thing still routing through the deprecated wrapper.
scope
write.sa.configs is untouched here. Deprecating it is the follow-up, and it needs get.results and run.sensitivity.analysis updated first or in the same change, otherwise SA analysis breaks in between. Sobol still needs its own pass to carry samples. sda/pda stay deferred.
testing
New tests cover the in-memory sampling path (loader and sampler called, do_ensemble FALSE, SA quantiles passed through), the pass-in path skipping both, the empty sa.samples guard, and that the design is built from the freshly sampled sa.samples. On the workflow side, tests cover the bundle being threaded into the generator and the deprecation warning firing only on auto-generation. The existing write.sa.configs integration test still passes against the new signature, confirming the SA path is unaffected.