Remove EnKFMain and introduce fixtures to workflows #7501

oyvindeide · 2024-03-21T13:42:29Z

Closes: #1804

Removes a lot of unneeded complexity, introduces fixtures to workflows.

PR title captures the intent of the changes, and is fitting for release notes.
Added appropriate release note label
Commit history is consistent and clean, in line with the contribution guidelines.
Make sure tests pass locally (after every commit!)

When applicable

When there are user facing changes: Updated documentation
New behavior or changes to existing untested code: Ensured that unit tests are added (See Ground Rules).
Large PR: Prepare changes in small commits for more convenient review
Bug fix: Add regression test for the bug
Bug fix: Create Backport PR to latest release

codecov-commenter · 2024-05-14T09:12:02Z

Codecov Report

Attention: Patch coverage is 90.27027% with 18 lines in your changes missing coverage. Please review.

Project coverage is 86.51%. Comparing base (37c4d20) to head (0819fb7).
Report is 2 commits behind head on main.

Files	Patch %	Lines
src/ert/shared/exporter.py	44.44%	10 Missing ⚠️
src/ert/libres_facade.py	66.66%	2 Missing ⚠️
src/ert/cli/main.py	0.00%	1 Missing ⚠️
src/ert/gui/tools/plugins/plugin.py	93.75%	1 Missing ⚠️
src/ert/gui/tools/plugins/plugin_handler.py	66.66%	1 Missing ⚠️
...rc/ert/gui/tools/run_analysis/run_analysis_tool.py	85.71%	1 Missing ⚠️
src/ert/run_models/base_run_model.py	88.88%	1 Missing ⚠️
src/ert/simulator/simulation_context.py	92.30%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #7501   +/-   ##
=======================================
  Coverage   86.51%   86.51%           
=======================================
  Files         383      383           
  Lines       23742    23760   +18     
  Branches      629      635    +6     
=======================================
+ Hits        20540    20557   +17     
- Misses       3126     3130    +4     
+ Partials       76       73    -3

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

src/ert/shared/share/ert/workflows/jobs/internal-gui/scripts/csv_export.py

oyvindeide · 2024-05-23T11:52:24Z

Had to force push due to conflicts to get the tests running

src/ert/config/ert_plugin.py

src/ert/config/ert_script.py

src/ert/libres_facade.py

src/ert/run_models/ensemble_smoother.py

src/ert/shared/hook_implementations/workflows/export_runpath.py

tests/unit_tests/all/plugins/test_export_runpath.py

sondreso

This looks good! The documentation also needs to be updated with a good description of the new behavior for workflow arguments.

src/ert/config/ert_script.py

sondreso · 2024-06-13T13:32:26Z

We should add documentation for the workflow plugins and their fixtures as well 📜

sondreso

Think this looks good now, only some minor comments!

Thanks for you hard work and persistence 🙏

docs/reference/workflows/workflows.rst

This removes the EnKFMain class, and replaces the remaning usage with pytest style fixtures. The current usage was mostly as a facade to ErtConfig, and it held state for the workflows.

oyvindeide marked this pull request as draft March 21, 2024 13:42

oyvindeide force-pushed the remove_enkf_main branch 7 times, most recently from dda27a6 to 9b9b45b Compare March 25, 2024 11:08

oyvindeide force-pushed the remove_enkf_main branch from 9b9b45b to 745cda8 Compare April 8, 2024 13:24

oyvindeide force-pushed the remove_enkf_main branch 2 times, most recently from 29c03fe to 035a9d3 Compare May 7, 2024 08:27

oyvindeide self-assigned this May 7, 2024

oyvindeide added the release-notes:breaking-change Automatically categorise as breaking change in release notes label May 7, 2024

oyvindeide force-pushed the remove_enkf_main branch 7 times, most recently from 350b643 to adc84bb Compare May 14, 2024 08:51

sondreso reviewed May 22, 2024

View reviewed changes

src/ert/shared/share/ert/workflows/jobs/internal-gui/scripts/csv_export.py Outdated Show resolved Hide resolved

oyvindeide force-pushed the remove_enkf_main branch from bec1759 to 0197014 Compare May 23, 2024 11:48

oyvindeide force-pushed the remove_enkf_main branch from 0197014 to e903090 Compare May 23, 2024 11:53

oyvindeide changed the title ~~Remove EnKFMain~~ Remove EnKFMain and introduce fixtures to workflows May 23, 2024

oyvindeide marked this pull request as ready for review May 23, 2024 12:32

sondreso mentioned this pull request Jun 3, 2024

Two different seeds are reported in the ERT log files #8024

Closed

oyvindeide force-pushed the remove_enkf_main branch 2 times, most recently from abb0707 to edbba6d Compare June 4, 2024 11:59