Include all numeric params enif#13286
Conversation
e44cb11 to
53560c5
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #13286 +/- ##
===========================================
- Coverage 90.05% 74.43% -15.62%
===========================================
Files 459 459
Lines 32087 32107 +20
===========================================
- Hits 28895 23899 -4996
- Misses 3192 8208 +5016
Flags with carried forward coverage won't be shown. Click here to find out more.
|
There was a problem hiding this comment.
Pull request overview
This PR updates the EnIF analysis workflow to build the regression/H-map (and related precision structure) using all numeric parameter groups from the experiment configuration, while still only persisting updates to update=True parameter groups.
Changes:
- Remove the explicit
parametersargument fromenif_update/analysis_EnIFand derive parameter groups from the experiment configuration instead. - Add
_load_numeric_parametersto load and filter parameter groups to numeric-only for EnIF computations. - Extend unit/UI/performance tests to cover inclusion of non-updateable numeric parameters in the regression inputs and update the new
enif_updatecall signature.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/ert/analysis/_enif_update.py |
Derives EnIF predictors from numeric parameter groups; updates storage behavior to only write updateable groups; removes parameters arg. |
src/ert/run_models/manual_update_enif.py |
Updates EnIF invocation to match new signature (no parameters=). |
src/ert/run_models/ensemble_information_filter.py |
Updates EnIF invocation to match new signature (no parameters=). |
tests/ert/unit_tests/analysis/test_enif_update.py |
Adds unit coverage for _load_numeric_parameters numeric filtering/casting behavior. |
tests/ert/ui_tests/cli/analysis/test_enif_update.py |
Adds UI-level coverage ensuring non-updateable numeric params contribute to regression but are not modified. |
tests/ert/performance_tests/test_obs_and_responses_performance.py |
Updates benchmark calls to enif_update to match the new signature. |
tests/ert/performance_tests/test_memory_usage.py |
Updates memory test call to enif_update to match the new signature. |
| def enif_update( | ||
| prior_storage: Ensemble, | ||
| posterior_storage: Ensemble, | ||
| observations: Iterable[str], | ||
| parameters: Iterable[str], | ||
| random_seed: int, |
There was a problem hiding this comment.
enif_update is exported from ert.analysis, and this change removes the public parameters argument entirely. If external callers relied on filtering parameters explicitly, this becomes a breaking API change; consider keeping parameters as an optional/keyword-only argument (defaulting to "all numeric") with a deprecation path, or ensure the change is documented/released as breaking.
53560c5 to
91f4a32
Compare
91f4a32 to
d9f430b
Compare
d9f430b to
7bffc16
Compare
Issue
Resolves #my_issue
Approach
Short description of the approach
(Screenshot of new behavior in GUI if applicable)
git rebase -i main --exec 'just rapid-tests')When applicable