Skip to content

Update documentation on reservoir simulators - #14318

Open
berland wants to merge 3 commits into
equinor:mainfrom
berland:update_ressim_docs
Open

Update documentation on reservoir simulators#14318
berland wants to merge 3 commits into
equinor:mainfrom
berland:update_ressim_docs

Conversation

@berland

@berland berland commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Explicitly state how NUM_CPU should be handled for Eclispe and Flow.

Fix backslash rendering bug for multiline Ert config examples

Fix erroneous example on supplying multiple options to Eclipse 100. Curly braces do not work.

Issue
Resolves gruff

Approach
🧠 ✍🏻

  • 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 'just rapid-tests')

When applicable

  • When screenshots are changed: Review screenshot-PR in ert-testdata,
    merge screenshot-PR in ert-testdata before merging this PR.
  • 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: Add backport label to latest release (format: 'backport release-branch-name')

@berland berland self-assigned this Aug 28, 2026
@berland berland added the release-notes:skip If there should be no mention of this in release notes label Aug 28, 2026
@berland berland added this to SCOUT Aug 28, 2026
@berland berland moved this to Ready for Review in SCOUT Aug 28, 2026
@berland
berland requested a review from oyvindeide August 28, 2026 13:02
@codecov-commenter

codecov-commenter commented Aug 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.74%. Comparing base (bf96534) to head (b5e20c0).
⚠️ Report is 8 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #14318      +/-   ##
==========================================
- Coverage   91.92%   91.74%   -0.19%     
==========================================
  Files         485      485              
  Lines       33655    34708    +1053     
==========================================
+ Hits        30939    31844     +905     
- Misses       2716     2864     +148     
Flag Coverage Δ
cli-tests 35.70% <ø> (-0.84%) ⬇️
fuzz 43.89% <ø> (-0.48%) ⬇️
gui-tests 58.17% <ø> (-0.46%) ⬇️
performance-and-unit-tests 81.04% <ø> (+0.09%) ⬆️
test 45.28% <ø> (-0.63%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...lugins/hook_implementations/forward_model_steps.py 94.59% <ø> (ø)

... and 14 files with indirect coverage changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Updates the forward model step documentation strings for reservoir simulators, aiming to clarify CPU handling (NUM_CPU vs parsing PARALLEL) and to correct/clarify configuration examples shown in generated docs.

Changes:

  • Expanded Eclipse 100 / Eclipse 300 documentation to explain how CPU count is determined via PARALLEL vs NUM_CPU.
  • Updated examples to fix multiline backslash rendering and to show multiple <OPTS> passed as a space-separated string (not curly braces).
  • Expanded Flow documentation to describe CPU selection behavior and forwarding multiple options via <OPTS>.
Suppressed comments (2)

src/ert/plugins/hook_implementations/forward_model_steps.py:320

  • Minor wording/grammar: use “set up” (verb) rather than “setup” in the description text.
Ert will parse the DATA-file for the :code:`PARALLEL` keyword and setup the
correct number of CPUs. If the :code:`PARALLEL` keyword is setup/computed
during the forward model, then :code:`NUM_CPU` must be set in the Ert config
for correct configuration of CPU usage.

src/ert/plugins/hook_implementations/forward_model_steps.py:397

  • Capitalization/terminology: “CPU” is typically uppercased in this context ("per CPU core").
returns. An appropriate setting for the threads parameter (per cpu core)

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/ert/plugins/hook_implementations/forward_model_steps.py
Comment thread src/ert/plugins/hook_implementations/forward_model_steps.py Outdated
@codspeed-hq

codspeed-hq Bot commented Aug 28, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 36 untouched benchmarks


Comparing berland:update_ressim_docs (b5e20c0) with main (4e36dd4)

Open in CodSpeed

Explicitly state how NUM_CPU should be handled for Eclipse and Flow.

Fix backslash rendering bug for multiline Ert config examples

Fix erroneous example on supplying multiple options to Eclipse 100. Curly braces
do not work.
@berland
berland force-pushed the update_ressim_docs branch from 6abe85b to f0420d3 Compare August 31, 2026 11:32
description="The Eclipse 300 compositional reservoir simulator from SLB",
description="""The Eclipse 300 compositional reservoir simulator from SLB.

Ert will parse the DATA-file for the :code:`PARALLEL` keyword and set up the

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggest linking to the DATA_FILE-keyword, as that is needed for us to parse it to read PARALLEL, same for ecl100.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed!

@ertomatic

Copy link
Copy Markdown
Collaborator

Screenshots differ from baselines. A baseline update PR has been prepared: equinor/ert-testdata#113

@berland

berland commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

Screenshots differ from baselines. A baseline update PR has been prepared: equinor/ert-testdata#113

False positive due to missing merge in equinor/ert-testdata#106

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport version-25.0 release-notes:skip If there should be no mention of this in release notes

Projects

Status: Ready for Review

Development

Successfully merging this pull request may close these issues.

5 participants