Move out the iterative ensemble smoother

[eivindjahren]

Adresses problem raised in #3708 where _ies is made a private module in ert. The public code is now part of the iterative_ensemble_smoother package.

Some of the nice testing done by the sloths team in the analysis milestone is lost as the c code no longer resides in one place. I have a created an additional issue: https://github.com/equinor/iterative_ensemble_smoother/issues/21 to rewrite these in python so that we can persist it.

• [x] Added appropriate release note label
• [x] PR title captures the intent of the changes, and is fitting for release notes.
• [x] Commit history is consistent and clean, in line with the contribution guidelines.

Adding labels helps the maintainers when writing release notes. This is the list of release note labels.

[codecov-commenter]

Codecov Report

Merging #3844 (cd3c984) into main (5f42627) will decrease coverage by 0.04%. The diff coverage is 71.42%.

@@            Coverage Diff             @@
##             main    #3844      +/-   ##
==========================================
- Coverage   63.86%   63.81%   -0.05%     
==========================================
  Files         595      592       -3     
  Lines       44582    44210     -372     
  Branches     4012     3854     -158     
==========================================
- Hits        28471    28213     -258     
- Misses      14769    14843      +74     
+ Partials     1342     1154     -188     

Impacted Files	Coverage Δ
src/clib/lib/analysis/analysis_module.cpp	25.00% <0.00%> (+0.15%)	:arrow_up:
src/clib/lib/analysis/update.cpp	40.48% <ø> (ø)
src/clib/lib/enkf/enkf_main.cpp	28.70% <ø> (+1.18%)	:arrow_up:
src/clib/lib/enkf/enkf_obs.cpp	59.33% <ø> (-0.10%)	:arrow_down:
src/ert/libres_facade.py	79.82% <50.00%> (ø)
src/clib/lib/analysis/ies/ies_config.cpp	33.33% <66.66%> (-19.17%)	:arrow_down:
src/ert/analysis/__init__.py	100.00% <100.00%> (ø)
src/ert/analysis/_es_update.py	92.12% <100.00%> (+0.45%)	:arrow_up:
src/ert/analysis/_ies/__init__.py	100.00% <100.00%> (+13.33%)	:arrow_up:
...rc/ert/shared/models/iterated_ensemble_smoother.py	88.77% <100.00%> (ø)
... and 48 more

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

[eivindjahren]

Testing on jenkins is currently failing because iterative_ensemble_smoother is not in bleeding. It will be as of midnight.

[dafeda]

A few comments (as discussed on Slack)

Ensemble Smoother is identical to Iterative Ensemble Smoother with step-length of 1.0, which means that IterativeEnsembleSmoother.update_step(step_length=1) is sufficient.

Row scaling or any type of localization should not be part of the package.

Personally, I prefer using variable names as defined in the paper instead of coming up new names. For example S instead of response_matrix. In this specific case, I would not know what to name S. Perhaps sensitivity matrix?

I would have preferred the tests to already have been moved before we start using this in production. Mostly because this kind of work tends to get delayed...

A link to the paper that defines the algorithm should be included in the readme.

[eivindjahren]

A few comments (as discussed on Slack)

Ensemble Smoother is identical to Iterative Ensemble Smoother with step-length of 1.0, which means that IterativeEnsembleSmoother.update_step(step_length=1) is sufficient.

Row scaling or any type of localization should not be part of the package.

Personally, I prefer using variable names as defined in the paper instead of coming up new names. For example S instead of response_matrix. In this specific case, I would not know what to name S. Perhaps sensitivity matrix?

I would have preferred the tests to already have been moved before we start using this in production. Mostly because this kind of work tends to get delayed...

A link to the paper that defines the algorithm should be included in the readme.

I have renamed the variable to sensitivity_matrix as suggested in https://github.com/equinor/iterative_ensemble_smoother/pull/22. Also renamed parameter_matrix to centered_anomaly_matrix as requested. The reason for these names is that they are part of the public API so you get problems of name collisions and the like. The name is still S everywhere, and discussed in the documentation.

As for row_scaling, it is described as an experimental feature of iterative_ensemble_smoother, which is a compromise that reduces complexity a lot. It means we can reduce the API surface to just python.

[dafeda]

Come to think of it, I think response_matrix is the correct term, but that it should be Y instead of S. Would have to check the paper to be certain though.

[eivindjahren]

We have now added back the c-tests as python tests. As these test the code being run, rather than a different c code-path, I consider this an improvement.

The tests should pass as soon as we get the 0.0.3 release out. I believe we are then ready for review.

[eivindjahren]

https://github.com/equinor/iterative_ensemble_smoother/issues/26

[eivindjahren]

https://github.com/equinor/iterative_ensemble_smoother/issues/27

[dafeda]

equinor/iterative_ensemble_smoother#27

Should we this before merging or do you want to wait?

[eivindjahren]

jenkins, test this please

[oysteoh]

Jenkins: Test this please!

[eivindjahren]

equinor/iterative_ensemble_smoother#27

Should we this before merging or do you want to wait?

I think it would be good to integrate first and simplify after. Simplifying now might confound potential problems with the existing solution.