Use new API from iterative_ensemble_smoother

[Blunde1]

Issue
Resolves #6474

Closes #5876

[Blunde1]

@dafeda @tommyod @kvashchuka I need some input.
I initially changed to new API with using the ESMDA module from iterative_ensemble_smoother. I then just avoided thinking and implementing the mapping from the ERT keyword IES_INVERSION to the inversion type needed by ESMDA.

Looking into it, I figured I should employ the SIES module for ES, and not ESMDA. I then used the mapping

0: "direct"
1: "subspace_exact"
2: "subspace_projected"
3: "subspace_projected"

But in doing this change, I also needed to include loading all parameters upon init of SIES object

 tot_num_params = sum(
      len(source_fs.experiment.parameter_configuration[key])
      for key in source_fs.experiment.parameter_configuration
  )
  param_groups = list(source_fs.experiment.parameter_configuration.keys())
  ensemble_size = ens_mask.sum()
  param_ensemble = _param_ensemble_for_projection(
      source_fs, iens_active_index, ensemble_size, param_groups, tot_num_params
  )

where param_ensemble is passed initially. I then became uncertain if it was okay to batch over parameters as we do with streaming. It did not feel "natural" using the API of SIES. So I reverted the changes and never pushed them towards the PR.

It does feel very natural using the ESMDA API. So my question is if you think we may just switch to this now, using the mapping

inversion_types = {0: "exact", 1: "subspace", 2: "subspace", 3: "subspace"}
inversion = inversion_types.get(module.ies_inversion, "Invalid input")

we then avoid loading all parameters, and have an algorithm and API that functions very elegantly with streaming of parameters. There is less code and I think the math is easier to grasp and understand that we are not messing up (avoiding the projection matrix + streaming -- the reason for passing all parameters on init I think).

This is what is currently suggested in the PR. What are you thoughts?

[Blunde1]

Note after talk with @dafeda
The switch is okay-ish if we can test that the result (snapshot) from ESMDA with alpha=1 (one iteration, no inflation) equals the result from SIES (one iteration, step-length=1) using the same noise for both.

If this is not already a test in iterative_ensemble_smoother, we should perhaps include it. Then we can do the change here without any fear.

[tommyod]

• Based on your feedback I have added a test that confirms that ESMDA equals SIES with one step. Note that when num_inputs < num_ensemble - 1, then Section (2.4.3) in the SIES paper triggers and the result is not identical.
• The snapshot tests were removed some time ago. We made several bugfixes that are backward-incompatible wrt the old snapshot tests, for instance centering E and multiplying with the prior X instead of X_i-1. I am not against adding back snapshot tests in general, but they won't match the results of the "old" snapshot tests.
• Overall I am quite confident that we produce the correct results, mainly owing to the ~3000 of parametrized property test that now run, compared to the ~25 before.

[Blunde1]

Note-to-self

SIES is also used in analysis_IES which takes an SEISargument, initialized and passed from IteratedEnsembleSmoother(BaseRunModel).

Suggested to-do changes.

• make SIES argument optional.
• • If it is not passed, then initialize it within analysis_IES.
• • Return it from analysis_IES and let IteratedEnsembleSmoother work on it / re-pass it in a new iteration
• The full parameter ensemble is needed upon initialization of the SIES object. Reflect this in _param_ensemble_for_projection
• Calculate step-lengths within IteratedEnsembleSmoother and then pass each step-length corresponding an iteration to the analysis_IES function. Employed then in SIES.sies_iteration

This should fix a lot

[dafeda]

I think this should resolve #5876 as well.

[Blunde1]

I think this should resolve #5876 as well.

Indeed it does. I am however not able to link the issue with the PR today 🤷 It is not coming up when searching for issues to close with this PR, and I cannot find the PR to link it from the issue. I am probably doing something wrong? If not, I will manually close it when this PR is merged.

[Blunde1]

In addition to snapshots. Only current problem seems to be that ERT (in cli?) uses SIES.iteration from the IES run model, and this iteration no longer exists. And in particular, the SIES object on the IES run-model is initially None, but created when first iteration starts.

I think it is more clean to define a @property on the IES run-model called sies_iteration and that implements appropriate logic. This is an alternative to current usage: run_model._w_container.iteration.

[Blunde1]

Some snapshot tests failing

In tests/unit_tests/analysis/test_es_update.py the tests

• test_update_snapshot is failing due to snapshot
• test_localization is failing only on updated parameters (snapshot)

remains to check

• test_that_surfaces_retain_their_order_when_loaded_and_saved_by_ert
• test_update_multiple_param

[Blunde1]

test_that_surfaces_retain_their_order_when_loaded_and_saved_by_ert fails from an exception in iterative_ensemble_smoother.

iterative_ensemble_smoother/esmda.py", line 256, in assimilate
      raise Exception("No more assimilation steps to run.")
  Exception: No more assimilation steps to run.

this is because we are using the high-level API, but also looping over parameters.
We need to use the low-level api.
This might be the case for adaptive esmda as well.

[Blunde1]

The test tests/unit_tests/cli/test_integration_cli.py::test_ies fails because difference in shapes on all parameters at initialization (3,5) and ensemble_mask (100,). The new API does not expect parameters to be subsetted at initialization.

[Blunde1]

• Failing tests in tests/unit_tests/analysis/test_es_update.py are all snapshots
• Failing test in tests/unit_tests/cli/test_integration_cli.py is a snapshot
• Failing test in tests/unit_tests/cli/test_model_hook_order.py is a strange mocking situation
• Failing tests in tests/unit_tests/gui/test_main_window.py are asserts that fail. Unknown cause

[Blunde1]

All tests are passing. Except manual snapshots

• Failing tests in tests/unit_tests/analysis/test_es_update.py are all snapshots
• Failing test in tests/unit_tests/cli/test_integration_cli.py is a snapshot

[Blunde1]

To fix failing snapshot tests, it must be possible to seed the update

[Blunde1]

We are no longer guaranteeing to get the exact same result for row scaling with 1.0 and ES, only that they are sampled correctly and that they both are reproducible.

[codecov-commenter]

Codecov Report

Attention: 6 lines in your changes are missing coverage. Please review.

Comparison is base (050d02a) 83.78% compared to head (1b0fbe1) 83.76%.
Report is 1 commits behind head on main.

Files	Patch %	Lines
src/ert/analysis/_es_update.py	90.19%	5 Missing ⚠️
src/ert/run_models/iterated_ensemble_smoother.py	91.66%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6634      +/-   ##
==========================================
- Coverage   83.78%   83.76%   -0.03%     
==========================================
  Files         365      365              
  Lines       21293    21297       +4     
  Branches      948      948              
==========================================
- Hits        17841    17839       -2     
- Misses       3158     3164       +6     
  Partials      294      294              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[tommyod]

Great now. The usage of the API looks good to be. I very much like the comments, making the code a bit easier to understand in the future.