Make restart on ES_MDA work with changed ensamble size#12909
Make restart on ES_MDA work with changed ensamble size#12909
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #12909 +/- ##
==========================================
- Coverage 90.16% 90.15% -0.01%
==========================================
Files 447 447
Lines 31057 31071 +14
==========================================
+ Hits 28002 28012 +10
- Misses 3055 3059 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
0799376 to
02132c9
Compare
02132c9 to
3f3d66d
Compare
|
Does this solve the issue? I think we need a larger gui test where we run ert with design_matrix realization 0-9, close ert and remove realization 8 and 9 from design matrix, open ert and try "Restart run" from the previous run. |
|
There should already be an integration test that you can backpack off, otherwise I can help you get started :) |
I'd say in this case, it should be enough to test it manually. The complexity of such a test (ie. restart esmda) would be much higher than the frequency of the actual cases that could have this issue. |
I am sure we have other tests that just create some mock storage of some kind. We only need to test this one panel or even just the validator. |
How I read the issue, its not about realizations from design matrix. Its about changing num_realizations in config to a lower value. Since ens_mask is loaded from prior_storage is has the old length. Could also be issues if we tamper with design matrix, I havent considered that since I didnt interpret the issue that way 🤷 |
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where restarting an ES_MDA run would fail when the ensemble size in the configuration was changed to a different value than the original run. The issue occurred because the code attempted to use bitwise AND (&=) on boolean arrays of different lengths, which is not supported.
Changes:
- Introduced
_create_combined_ensemble_maskfunction that handles mask intersection using set operations on indices instead of array operations, allowing masks of different lengths to be properly combined - Updated
smoother_updateto use the new function instead of in-place bitwise AND - Added comprehensive unit tests covering various scenarios including different-length masks
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/ert/analysis/_es_update.py |
Added _create_combined_ensemble_mask function to safely intersect ensemble masks of different lengths; refactored smoother_update to use the new function |
tests/ert/unit_tests/analysis/test_es_update.py |
Added import for the new function and parametrized unit tests covering None case, same-length intersection, different-length intersection, and no-intersection scenarios |
5363dfb to
25f4704
Compare
|
Looks good. Only comment is the testcase suggested by copilot. The failing codspeed test is probably just flaky |
25f4704 to
af2c778
Compare
Issue
Resolves #12180
Approach
Created a function for re-mapping lists into sets, finding common indices and converting back to list for further processing. Comparing sets eliminates the problem of different length lists.
(Screenshot of new behavior in GUI if applicable)
git rebase -i main --exec 'just rapid-tests')When applicable