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

Add dev documentation on everest vs ert data models #9820

Conversation

StephanDeHoop
Copy link
Contributor

@StephanDeHoop StephanDeHoop commented Jan 21, 2025

Issue
Resolves #9619

Approach
Attempts to clarify the differences between ERT and EVEREST data models. There are things in here that need revision (like the mapping, these equations work but not sure if it makes sense in all cases of optimization meaning gradient vs. derrivative free methods etc.). Also, with Yngve we suggest a couple of name changes:

  • simulation -> ert_realization (at least in the code, since they are synonymous)
  • geo_realization -> model_realization (or just realization towards the user, we are not just concerned with GEO apps?)
  • <GEO_ID> - > <STATIC_MODEL_ID> or <MODEL_REALIZATION_ID>

(Screenshot of new behavior in GUI if applicable)

  • 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 unit tests pass locally after every commit (git rebase -i main --exec 'pytest tests/ert/unit_tests -n auto --hypothesis-profile=fast -m "not integration_test"')

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

Copy link

codspeed-hq bot commented Jan 21, 2025

CodSpeed Performance Report

Merging #9820 will not alter performance

Comparing StephanDeHoop:add_dev_documentation_on_everest_vs_ert_data_models (b0302e4) with main (210f25b)

Summary

✅ 25 untouched benchmarks

Copy link
Contributor

@verveerpj verveerpj left a comment

Choose a reason for hiding this comment

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

In principle correct. But some amendments.

docs/everest/development.rst Outdated Show resolved Hide resolved
docs/everest/development.rst Outdated Show resolved Hide resolved
@StephanDeHoop StephanDeHoop marked this pull request as ready for review February 5, 2025 09:30
@verveerpj
Copy link
Contributor

Just a general remark: I am not happy with the use of geo, <GEO> and geo realization. That ties the optimizer unnecessarily to geological applications. Considering that the domain should (at least) be energy / energy transition, we should probably be consistent and talk about model realizations rather than geo realizations.

@StephanDeHoop
Copy link
Contributor Author

StephanDeHoop commented Feb 6, 2025

Just a general remark: I am not happy with the use of geo, <GEO> and geo realization. That ties the optimizer unnecessarily to geological applications. Considering that the domain should (at least) be energy / energy transition, we should probably be consistent and talk about model realizations rather than geo realizations.

Yes I completely agree, that's why I suggested in the description of the PR to change this name. However, since it is not changed in the code yet, I thought we should be consistent with what we currently have until we decide on a better name, like model_realizations (and MODEL_ID instead of GEO_ID). If you think it's OK to name it already then I'll do that and make another quick PR doing this name change (at least in the code, we can then think about if we should also rename the actual folder name to where we write the output).

Copy link
Contributor

@yngve-sk yngve-sk left a comment

Choose a reason for hiding this comment

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

Nice job, I think this looks really nice now, and feel like someone new to Everest/ERT could read this and get a better idea of what's going on. LGTM, I left some smaller comments I think you can address before merging.

@StephanDeHoop StephanDeHoop force-pushed the add_dev_documentation_on_everest_vs_ert_data_models branch from 8543d18 to b0302e4 Compare February 10, 2025 12:10
@StephanDeHoop StephanDeHoop merged commit ef417b6 into equinor:main Feb 10, 2025
27 checks passed
@StephanDeHoop StephanDeHoop deleted the add_dev_documentation_on_everest_vs_ert_data_models branch February 10, 2025 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Document the data model for everest
3 participants