Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify simulation arguments to run models #8140

Merged
merged 10 commits into from
Jun 21, 2024

Conversation

oyvindeide
Copy link
Collaborator

@oyvindeide oyvindeide commented Jun 12, 2024

The intention here is to reduce the statefullness of the run models, by reducing the use of simulation arguments as holders of state, and internalizing state on the run models instead. Also to simplify the simulation arguments by removing a lot of unneeded properties.

This results in a breaking change for the cli, which aligns it with the GUI in terms of naming the ensembles in ensemble smoother.

Closes #7633

  • 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

@oyvindeide oyvindeide self-assigned this Jun 12, 2024
@oyvindeide oyvindeide added the release-notes:maintenance Automatically categorise as maintenance change in release notes label Jun 12, 2024
@oyvindeide oyvindeide force-pushed the remove_current branch 3 times, most recently from 448e946 to 3031204 Compare June 13, 2024 13:56
@oyvindeide oyvindeide changed the title Remove current case from simulation arguments Simplify simulation arguments to run models Jun 13, 2024
@oyvindeide oyvindeide force-pushed the remove_current branch 4 times, most recently from 1727479 to 040a5f2 Compare June 14, 2024 10:35
@codecov-commenter
Copy link

codecov-commenter commented Jun 14, 2024

Codecov Report

Attention: Patch coverage is 91.07143% with 5 lines in your changes missing coverage. Please review.

Project coverage is 86.56%. Comparing base (f29176b) to head (e238e0d).

Files Patch % Lines
src/ert/run_models/multiple_data_assimilation.py 81.25% 3 Missing ⚠️
src/ert/cli/main.py 0.00% 1 Missing ⚠️
src/ert/storage/local_storage.py 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8140      +/-   ##
==========================================
- Coverage   86.62%   86.56%   -0.06%     
==========================================
  Files         384      384              
  Lines       23831    23773      -58     
  Branches      618      627       +9     
==========================================
- Hits        20644    20580      -64     
- Misses       3107     3112       +5     
- Partials       80       81       +1     

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

@oyvindeide oyvindeide added the release-notes:breaking-change Automatically categorise as breaking change in release notes label Jun 14, 2024
@oyvindeide oyvindeide force-pushed the remove_current branch 7 times, most recently from 4de603f to 5a9465a Compare June 20, 2024 11:36
@oyvindeide
Copy link
Collaborator Author

Semeio failure is expected, fix here: equinor/semeio#624

Copy link
Contributor

@dafeda dafeda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice

@oyvindeide oyvindeide enabled auto-merge (rebase) June 21, 2024 08:25
@oyvindeide oyvindeide merged commit 4404e4b into equinor:main Jun 21, 2024
37 of 38 checks passed
@oyvindeide oyvindeide deleted the remove_current branch June 21, 2024 08:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes:breaking-change Automatically categorise as breaking change in release notes release-notes:maintenance Automatically categorise as maintenance change in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

False positive on running in existing runpath
4 participants