enable tuning postprocessors #966
Conversation
R/grid_helpers.R
Outdated
| # postprocessor parameters while iterating in the model grid | ||
| # TODO: will this introduce issues when there are matching postprocessor | ||
| # values across models? | ||
| # ... i think we actually want to (temporarily?) situate these as submodels |
There was a problem hiding this comment.
My first pass at enabling the submodel trick—nest by the postprocessor and unnest later—but that doesn't quite do the job.
There was a problem hiding this comment.
that doesn't quite do the job.
Can you expand on this?
There was a problem hiding this comment.
See the linked notes above!
R/min_grid.R
Outdated
| # See: https://gist.github.com/simonpcouch/28d984cdcc3fc6d22ff776ed8740004e | ||
| nest_min_grid <- function(min_grid, post_params) { | ||
| # TODO | ||
| min_grid |
There was a problem hiding this comment.
I thiiink this will be our entry point; patch min_grid() output by dropping postprocessor values into the nested .submodel structure.
There was a problem hiding this comment.
The postprocessing object will only be available from within a workflow.
We could make a workflow method for min_grid() so that we have all of the information coming from a single object.
It might be possible to execute min_grid() on the model spec then generate a separate (but similar) data structure from the postprocessor, then join the two.
There was a problem hiding this comment.
I wonder if a join, like you suggest, would be the more effective way to make this happen. Right now, my theory is that we can take the output from model_spec min_grid() methods and do some dplyr/tidyr-fu based on the postprocessor parameter names.
This is gnaaaarly, though.
| paste0( | ||
| iter_config, | ||
| "_Postprocessor", | ||
| iter_postprocessor |
There was a problem hiding this comment.
Still needs a format_with_padding().
an attempt to enable the submodel trick with postprocessors. doesn't quite do the trick--see the newly added but skipped test.
|
Spent a bit with 4c763ed in an attempt to enable the submodel trick but still not quite there. None of the tuning code now works from this PR as I was focused on |
| ) | ||
| ) %>% | ||
| select(-.iter_config) %>% | ||
| nest(post = c(any_of(post_params), ".iter_postprocessor", ".msg_postprocessor", ".iter_config_post")) |
There was a problem hiding this comment.
We actually want postprocessor parameters names both inside and outside of .submodels, but this results in them only being inside of .submodels.
Heads up--this PR is not yet functional!
Enables tuning arguments in tailor postprocessors. Works in some cases, but still fails most others. Importantly, this disables the submodel trick right now and will need to be addressed before this is ready for review. Here are some notes about what's so tricky there.