Implement parsing of commonly configured summary observations by SAKavli · Pull Request #13438 · equinor/ert

SAKavli · 2026-04-29T08:38:44Z

This change will allow users to define summary observations in bulk, defined under a single well name sharing the well name and localization values.

This change has one assumption that there is only one BREAKTHROUGH observation per well. This might need to be revisited.

The naming of the WELL object should preferably be just WELL but it shadows the keyword of RFT observations WELL, so the well object name is currently named WELL_NAME for now.

Issue
Resolves #12943

All the summary observations in snake_oil can now have a shared configuration like:

SUMMARY {
    VALUES = summary_values.csv;
    WELL OP1 {
        LOCALIZATION {
            EAST = 10;
            NORTH = 20;
            RADIUS = 2000;
        };
    };
};

With csv file:

well, keyword, value, error, date
OP1, WOPR, 0.1, 0.05, 2010-03-31
OP1, WOPR, 0.7, 0.07, 2010-12-26
OP1, WOPR, 0.5, 0.05, 2011-12-21
OP1, WOPR, 0.3, 0.075, 2012-12-15
OP1, WOPR, 0.2, 0.035, 2013-12-10
OP1, WOPR, 0.015, 0.01, 2015-03-15

Approach
Short description of the approach

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

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: Add backport label to latest release (format: 'backport release-branch-name')

codecov-commenter · 2026-04-29T08:51:26Z

Codecov Report

❌ Patch coverage is 92.92035% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.96%. Comparing base (eb7925e) to head (d29ee0a).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
src/ert/config/_observations.py	91.57%	8 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #13438      +/-   ##
==========================================
- Coverage   89.97%   89.96%   -0.01%     
==========================================
  Files         460      460              
  Lines       32409    32517     +108     
==========================================
+ Hits        29160    29254      +94     
- Misses       3249     3263      +14

Flag	Coverage Δ
cli-tests	`37.02% <32.74%> (-0.03%)`	⬇️
fuzz	`43.97% <26.54%> (-0.06%)`	⬇️
gui-tests	`66.45% <32.74%> (-0.05%)`	⬇️
performance-and-unit-tests	`78.02% <92.92%> (+0.03%)`	⬆️
test	`45.62% <25.66%> (-0.10%)`	⬇️

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

Files with missing lines	Coverage Δ
src/ert/config/parsing/observations_parser.py	`93.63% <100.00%> (+1.16%)`	⬆️
src/ert/config/_observations.py	`94.83% <91.57%> (-0.77%)`	⬇️

... and 2 files with indirect coverage changes

codspeed-hq · 2026-04-29T09:27:56Z

Merging this PR will not alter performance

✅ 36 untouched benchmarks

_{Comparing SAKavli:add-common-summary-configuration (d29ee0a) with main (15b1b96)}

Copilot

Pull request overview

Adds support for a new SUMMARY { ... } observation declaration that allows defining multiple SUMMARY_OBSERVATIONs from a CSV plus shared well-level settings (localization and optional breakthrough), integrating this into the existing observations parsing/validation pipeline.

Changes:

Extend the observations grammar to parse a SUMMARY block with nested WELL_NAME { ... } and BREAKTHROUGH { ... }.
Implement SummaryObservation.from_common_config_dict(...) to expand the common config into concrete SummaryObservation (and BreakthroughObservation) instances.
Add unit tests verifying equivalence between common-config parsing and equivalent explicit observation declarations, plus a validation/error-labeling test.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 8 comments.

File	Description
`src/ert/config/parsing/observations_parser.py`	Extends the observation grammar and transformer to recognize `SUMMARY`, `WELL_NAME`, and nested `BREAKTHROUGH` blocks.
`src/ert/config/_observations.py`	Expands `SUMMARY` common config into observation instances, adds CSV-column missing error helper, and factors localization shape registration into a helper.
`tests/ert/unit_tests/config/test_summary_config.py`	Refactors test helpers and adds tests for the new `SUMMARY` common configuration behavior and error surfacing.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 10 comments.

Comments suppressed due to low confidence (1)

src/ert/config/parsing/observations_parser.py:109

The "unknown observation type" error message is now triggered for both TYPE and COMMON_TYPE, but the message still only lists the legacy observation types. Update the message to include SUMMARY (and consider including BREAKTHROUGH_OBSERVATION too) so users see the full set of accepted top-level keywords.

                token=unexpected_token,
            ), ["COMMON_TYPE", "TYPE"]:
                message = (
                    f"Unknown observation type '{unexpected_token}', "
                    f"expected either 'RFT_OBSERVATION', 'GENERAL_OBSERVATION', "
                    f"'SUMMARY_OBSERVATION' or 'HISTORY_OBSERVATION'."
                )

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (1)

src/ert/config/parsing/observations_parser.py:109

The "Unknown observation type" error message is now out of sync with the grammar: it still lists only RFT/GENERAL/SUMMARY_OBSERVATION/HISTORY_OBSERVATION, but the parser also accepts BREAKTHROUGH_OBSERVATION and the new SUMMARY common config. Updating this message would make validation errors accurate and less confusing.

                message = (
                    f"Unknown observation type '{unexpected_token}', "
                    f"expected either 'RFT_OBSERVATION', 'GENERAL_OBSERVATION', "
                    f"'SUMMARY_OBSERVATION' or 'HISTORY_OBSERVATION'."
                )

SAKavli · 2026-05-05T12:22:14Z

Should change "common" to "shared" configuration

Copilot

Pull request overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated 8 comments.

Comments suppressed due to low confidence (1)

src/ert/config/parsing/observations_parser.py:108

The unknown-observation-type error message is now out of sync with the grammar: SUMMARY shared blocks are valid and BREAKTHROUGH_OBSERVATION is also a supported TYPE, but the message only lists a subset. Update the message to include SUMMARY and BREAKTHROUGH_OBSERVATION (and any other supported types) so users get accurate guidance.

            ), ["SHARED_TYPE", "TYPE"]:
                message = (
                    f"Unknown observation type '{unexpected_token}', "
                    f"expected either 'RFT_OBSERVATION', 'GENERAL_OBSERVATION', "
                    f"'SUMMARY_OBSERVATION' or 'HISTORY_OBSERVATION'."

Copilot

Pull request overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (1)

src/ert/config/parsing/observations_parser.py:109

The "unknown observation type" parse error message is now triggered for both TYPE and SHARED_TYPE tokens, but the message does not mention the new "SUMMARY" shared-config keyword (and it also omits BREAKTHROUGH_OBSERVATION). This makes the validation error misleading when users mistype/encounter SUMMARY-related parsing issues. Update the expected-type list in the message to match the grammar/token expectations.

            case UnexpectedToken(
                token=unexpected_token,
            ), ["SHARED_TYPE", "TYPE"]:
                message = (
                    f"Unknown observation type '{unexpected_token}', "
                    f"expected either 'RFT_OBSERVATION', 'GENERAL_OBSERVATION', "
                    f"'SUMMARY_OBSERVATION' or 'HISTORY_OBSERVATION'."
                )

eivindjahren · 2026-05-08T12:35:33Z

+            validated_error = validate_positive_float(
+                row["error"], f"({summary_key}) ERROR", strictly_positive=True
+            )
+            validated_value = validate_float(row["value"], f"({summary_key}) VALUE")
+            date = row["date"]


This should probably be fixed.

eivindjahren · 2026-05-08T12:31:52Z

+BREAKTHROUGH allows the user to define a BREAKTHROUGH observations for
+the given well. BREAKTHROUGH is configured like a regular
+BREAKTHROUGH_OBSERVATION - which requires the fields KEY, THRESHOLD, DATE
+and ERROR. BREAKTHROUGH will also inherit the LOCALIZATION values should
+they be defined for the well. Only one occurrence of BREAKTHROUGH can
+be configured per WELL.


This seems relevant :)

eivindjahren · 2026-05-08T12:35:12Z

+        csv_df = pl.read_csv(
+            csv_path,
+            encoding="utf-8",
+        )


This should probably be fixed

+@given(
+    keyword=st.text(
+        min_size=1,
+        max_size=8,
+        alphabet=st.characters(categories=("L", "N", "P"), exclude_characters=","),
+    )
+)
+@pytest.mark.usefixtures("use_tmpdir")
+def test_that_fully_populated_csv_does_not_crash_given_arbitrary_keyword(keyword):


eivindjahren · 2026-05-08T12:34:49Z

+Shared configuration for summary observations
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Summary observations can also be created in bulk by the "SUMMARY"


Maybe using the word "bulk" instead of "Shared" throughout, makes this easier to understand.

eivindjahren · 2026-05-08T12:35:58Z

    ObservationType.GENERAL: GeneralObservation,
    ObservationType.RFT: RFTObservation,
    ObservationType.BREAKTHROUGH: BreakthroughObservation,
+    ObservationType.SUMMARY_SHARED_CONFIG: SummaryObservation,


So maybe BULK_SUMMARY ?

eivindjahren

I mostly had comments about using "bulk" instead of "shared" for naming and also some of the errors noted by copilot needs to be adressed. Otherwise, this looks ready to go :)

This change will allow users to define summary observations in bulk, defined under a single well name sharing the well name and localization values. This change has one assumption that there is only one BREAKTHROUGH observation per well. This might need to be revisited. The naming of the WELL object should preferably be just WELL but it shadows the keyword of RFT observations WELL, so the well object name is currently named WELL_NAME for now.

SAKavli added the release-notes:new-feature Automatically categorise as new feature in release notes label Apr 29, 2026

SAKavli force-pushed the add-common-summary-configuration branch from be7bd7e to 4f0d418 Compare April 29, 2026 10:04

SAKavli requested a review from Copilot April 29, 2026 10:05

Copilot started reviewing on behalf of SAKavli April 29, 2026 10:06 View session

Copilot AI reviewed Apr 29, 2026

View reviewed changes

SAKavli force-pushed the add-common-summary-configuration branch 3 times, most recently from 40f3b3c to 79b9656 Compare April 30, 2026 12:00

SAKavli requested a review from Copilot April 30, 2026 12:00

Copilot started reviewing on behalf of SAKavli April 30, 2026 12:00 View session

Copilot AI reviewed Apr 30, 2026

View reviewed changes

SAKavli force-pushed the add-common-summary-configuration branch 7 times, most recently from bd7ea4d to 5df41ea Compare April 30, 2026 14:28

SAKavli requested a review from Copilot April 30, 2026 14:37

Copilot AI reviewed Apr 30, 2026

View reviewed changes

Comment thread src/ert/config/_observations.py Outdated

Comment thread src/ert/config/_observations.py Outdated

Comment thread src/ert/config/_observations.py Outdated

Comment thread src/ert/config/_observations.py Outdated

Comment thread src/ert/config/_observations.py

SAKavli force-pushed the add-common-summary-configuration branch 8 times, most recently from 1996808 to e8b8fa4 Compare May 4, 2026 12:40

SAKavli force-pushed the add-common-summary-configuration branch 4 times, most recently from a99b64d to 31e009e Compare May 5, 2026 12:07

SAKavli force-pushed the add-common-summary-configuration branch from 8b60810 to c89fc07 Compare May 8, 2026 08:40

Extract summary default name method

6fd1ba0

SAKavli force-pushed the add-common-summary-configuration branch 5 times, most recently from c8b2e1d to 2fc76bd Compare May 8, 2026 10:21

SAKavli requested a review from Copilot May 8, 2026 10:41

Copilot started reviewing on behalf of SAKavli May 8, 2026 10:41 View session

Copilot AI reviewed May 8, 2026

View reviewed changes

SAKavli force-pushed the add-common-summary-configuration branch 2 times, most recently from dce2245 to 24df41c Compare May 8, 2026 11:52

SAKavli requested a review from Copilot May 8, 2026 11:56

Copilot started reviewing on behalf of SAKavli May 8, 2026 11:57 View session

Copilot AI reviewed May 8, 2026

View reviewed changes

SAKavli force-pushed the add-common-summary-configuration branch from 24df41c to d29ee0a Compare May 8, 2026 12:20

eivindjahren reviewed May 8, 2026

View reviewed changes

eivindjahren approved these changes May 8, 2026

View reviewed changes

SAKavli added 2 commits May 8, 2026 15:55

Add summary shared configuration documentation

04e79a5

SAKavli force-pushed the add-common-summary-configuration branch from d29ee0a to 04e79a5 Compare May 8, 2026 13:55

Conversation

SAKavli commented Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

When applicable

Uh oh!

codecov-commenter commented Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

codspeed-hq Bot commented Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will not alter performance

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

SAKavli commented May 5, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

eivindjahren May 8, 2026

Choose a reason for hiding this comment

Uh oh!

eivindjahren May 8, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

eivindjahren May 8, 2026

Choose a reason for hiding this comment

Uh oh!

eivindjahren May 8, 2026

Choose a reason for hiding this comment

Uh oh!

SAKavli commented Apr 29, 2026 •

edited

Loading

codecov-commenter commented Apr 29, 2026 •

edited

Loading

codspeed-hq Bot commented Apr 29, 2026 •

edited

Loading