Add summary observations to yaml converter

[SAKavli]

The purpose of this converter class is to support conversion of observation configurations to a format supported by webviz.

For now, only summary observations are of interest. RFTs are manually loaded through other workflows and other observations are not of interest as of now.

Localization is not supported in webviz, so those attributes are left out.

Issue
Resolves #14042

[Copilot]

Pull request overview

Adds a new observation converter that exports ERT summary observations to a Webviz-compatible YAML structure, and wires it into the existing convert_observations CLI conversion dispatcher.

Changes:

• Introduce YamlConverter to serialize summary observations into summary_observations.yaml using ruamel.yaml.
• Register yaml as a supported conversion format in the observation converter dispatcher.
• Add unit tests validating file output, error cases, grouping behavior, and “do not overwrite” behavior.

Reviewed changes

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

File	Description
src/ert/observation_converters/summary_to_yaml.py	New YAML export implementation for summary observations and a CLI entrypoint function.
src/ert/observation_converters/dispatcher.py	Adds yaml to supported formats and routes it to the new converter.
tests/ert/unit_tests/cli/test_summary_to_yaml.py	New unit tests covering YAML export behavior and CLI integration.

Comments suppressed due to low confidence (1)

tests/ert/unit_tests/cli/test_summary_to_yaml.py:105

• This test has the same well=None problem as the previous one: it will generate keys containing "None" and the {k1, k2} assertion will fail. Use non-None wells (or construct observations with explicit keys) and assert against the produced keys.

    Path("summary_observations.yaml").write_text("existing", encoding="utf-8")
    assert Path("summary_observations.yaml").is_file()

    converter = YamlConverter(observations=observations)
    with pytest.raises(

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.86%. Comparing base (a9c4e65) to head (164caa9).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #14057      +/-   ##
==========================================
+ Coverage   91.84%   91.86%   +0.01%     
==========================================
  Files         483      484       +1     
  Lines       33478    33543      +65     
==========================================
+ Hits        30747    30813      +66     
+ Misses       2731     2730       -1     

Flag	Coverage Δ
cli-tests	36.17% <42.85%> (+0.01%)	⬆️
fuzz	44.08% <42.85%> (-0.01%)	⬇️
gui-tests	58.39% <42.85%> (-0.04%)	⬇️
performance-and-unit-tests	80.45% <100.00%> (-0.04%)	⬇️
test	46.33% <0.00%> (-0.09%)	⬇️

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

Files with missing lines	Coverage Δ
src/ert/__main__.py	88.30% <ø> (ø)
src/ert/observation_converters/dispatcher.py	100.00% <100.00%> (ø)
src/ert/observation_converters/summary_to_yaml.py	100.00% <100.00%> (ø)

... and 4 files with indirect coverage changes

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (1)

src/ert/observation_converters/summary_to_yaml.py:50

• _summary_to_yaml_dict() iterates over summary_keys built from every observation key, so if there are multiple observations for the same summary key you end up with duplicated {"key": ...} entries in the YAML output (one per observation), each containing the full list of observations for that key. This breaks the intended Webviz format (one entry per key) and will also make the snake_oil happy-path test produce repeated blocks.

    def _summary_to_yaml_dict(self) -> YamlDict:
        summary_observations = [
            o for o in self.summary_observations if o.type == "summary_observation"
        ]
        summary_keys: list[str] = [o.key for o in summary_observations]
        summary_list: list[SummaryDict] = []
        for key in summary_keys:
            observations_with_key = [o for o in summary_observations if o.key == key]
            obs_dicts: list[YamlObservation] = [

[SAKavli]

Warnings from loading the Ert config file is printed to terminal. I will make a fixup commit for this.

[codspeed-hq]

Merging this PR will not alter performance

✅ 36 untouched benchmarks

---

_{Comparing SAKavli:add-yaml-obs-converter (164caa9) with main (28f6406)}

[ajaust]

Nice changes. I found some potential issues. Please have a look. 🙂

[SAKavli]

I force pushed the ErtConfig changes to the single commit before reviewing your comments, @ajaust .
I then fixed all your comments in fixups afterwards to squash.
All were very valid, thanks for a thorough review!

[Copilot]

Pull request overview

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

Suppressed comments (3)

src/ert/observation_converters/summary_to_yaml.py:57

• _summary_to_yaml_dict() does an O(N×K) scan: for each unique summary key it re-filters self.summary_observations to collect matching observations. For larger observation configs this can become unnecessarily slow; group observations by key in a single pass and then sort within each group.

        summary_keys: set[str] = {str(o["KEY"]) for o in self.summary_observations}
        summary_list: list[SummaryDict] = []
        for key in natsorted(summary_keys):
            observations_with_key = [
                o for o in self.summary_observations if o["KEY"] == key

src/ert/observation_converters/summary_to_yaml.py:86

• In convert_summary_to_yaml(), config_dict is None can never be true (it is always a dict), and the local name file shadows the built-in. This also means the code doesn’t explicitly guard against obs_config being None if the dict entry is malformed. Rename the variables and tighten the check.

    file, obs_config = config_dict.get("OBS_CONFIG", (None, None))

    if file is None or config_dict is None:
        raise ErtCliError("No observation configuration found.\nExiting ...")

    yaml_exporter = YamlConverter(
        observations=obs_config,
    )

tests/ert/unit_tests/cli/test_summary_to_yaml.py:140

• This comment says the setup "expects" a SUMMARY/forward-model ConfigWarning to be raised, but convert_observations(..., format='yaml') currently only calls ErtConfig._config_dict_from_contents() and won’t instantiate SummaryConfig (where the warning is emitted). Either adjust the wording or change the test to exercise the warning-producing code path.

    # This setup expects the warning:
    # 'Config contains a SUMMARY key but no forward model steps'
    # to be raised

[ajaust]

Great! I marked all my previous comments as resolved. I have one minor new comment and one potentially larger, but the larger comment needs your expertise.

[ajaust]

I don't think we need to check for the config_dict. ErtConfig._config_dict_from_contents always returns a ConfigDict and line 75 would have failed already if the config_dict would have been `None.

Suggested change

	if file is None or config_dict is None:
	if file is None:

Maybe you meant to check the obs_config?

Suggested change

	if file is None or config_dict is None:
	if file is None or obs_config is None:

[ajaust]

There are some potential issues flagged by GitHub Copilot CLI that I want to raise because I need your opinion. I just don't know enough.

The issues mostly stem from the fact that we are using read_file over ErtConfig.from_file and therefore use the ObservationDict instead of Observation type. This also implies that the yaml converter works differently from the summary to bulk converter which uses ErtConfig.from_file:

1. Loading the config file via ErtConfig.from_file automatically checks for HISTORY_OBSERVATION and raises an error in this case. The YAML converter skips this check and therefore convert the observation differently than the bulk converter.
2. Using ErtConfig.from_file checks for the existence of the DATE keyword. If are converting a RESTART-based SUMMARY_OBSERVATION the conversion "date": str(o["DATE"]), may fail. I the DATE is missing if there is a RESTART.
3. Using ErtConfig.from_file converts errors into their absolute value based on the actual ERROR_MODE. We directly write the error value "error": float(o["ERROR"]) without checking the ERROR_MODE. If ERROR_MODE is anything else that ABS the written error value may be wrong.

[SAKavli]

Excellent feedback @ajaust ! ⭐

1. At first I was not concerned with any other observations types as they will all be discarded asap either way. History observations should probably be converted to summary - as they are basically a timeseries of summary observations, but these have been deprecated for half a year, so I think I won't handle these.
2. DATE is not a valid keyword, but you are right that I should have a test to make sure invalid keywords or missing valid keywords are handled correctly. I will make a fixup for this.
3. This I haven't thought of yet, I will see what can be done about this, good catch!

[ajaust]

Thanks for the explanation. Just some

1. I think Copilot's main concern was that the summary-to-bulk converter will issue a warning if it sees a HISTORY_OBSERVATION and skip the HISTORY_OBSERVATION. The yaml converter may skip it, but it will not issue the warning.
2. Then, I (and Copilot) must have misunderstood something. I expected that the o.date key in the dictionary, see https://github.com/SAKavli/ert/blob/74e26dd0713c7eade1f1a890b62310681e373d64/src/ert/observation_converters/summary_to_yaml.py#L47, comes from a DATE keyword.
3. Maybe we can just start by adding some tests using the different ERROR_MODEs and to check if the tests fail.

[SAKavli]

I completely forgot about error modes while trying to fix the ErtConfig.from_file issue. I will make tests 👍

I am not sure if I understand your concern about restart. The workflow will fail when creating the summary observations:

                case "DAYS" | "HOURS" | "RESTART":
                    raise ObservationConfigError.with_context(
                        (
                            "SUMMARY_OBSERVATION must use DATE to specify "
                            "date, DAYS | HOURS is no longer allowed. "
                            "Please run:\n ert convert_observations "
                            "<your_ert_config.ert>\nto migrate the observation config "
                            "to use the correct format."
                        ),
                        key,
                    )

Do you want a test for this - or what was the concern?

[ajaust]

Everything is fine now. When we were using a different code path for reading the config, I was not 100% sure that we will fail with the linked error message. With the current code path via ErtConfig.with_plugins.from_file I am sure that it work well.

I leave it to you if you want to add another test. I am happy the way it is. 👍

[Copilot]

Pull request overview

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

Suppressed comments (3)

src/ert/observation_converters/summary_to_yaml.py:63

• _summary_to_yaml_dict() currently re-filters self.summary_observations for every key (quadratic behavior) and also mutates o.date in-place when stripping T00:00:00. This can become slow for large observation sets and introduces side effects on Observation objects that may be reused elsewhere. Group observations by key once, and format the date in the exported dict without mutating the original objects.

        summary_keys: set[str] = {o.key for o in self.summary_observations}
        summary_list: list[SummaryDict] = []
        for key in natsorted(summary_keys):
            observations_with_key = [
                o for o in self.summary_observations if o.key == key

tests/ert/unit_tests/cli/test_summary_to_bulk.py:411

• This test assumes get_site_plugins().installed_workflow_jobs is non-empty; if it's empty in some environments, next(iter(...)) raises StopIteration and the test fails for reasons unrelated to the behavior under test. Add a guard to skip when no workflow jobs are available.

    site_plugins = get_site_plugins()

    arbitrary_existing_job = next(iter(site_plugins.installed_workflow_jobs))

src/ert/observation_converters/summary_to_yaml.py:83

• convert_summary_to_yaml() currently ignores all warnings while parsing the config. That risks suppressing unrelated ConfigWarnings (and other warnings) that users should see. Prefer filtering only the specific warning you expect (e.g. the SUMMARY-without-simulator warning) instead of globally ignoring everything.

    with warnings.catch_warnings():
        warnings.filterwarnings(action="ignore")
        ert_config = ErtConfig.with_plugins(site_plugins).from_file(config)

[ajaust]

Nice work. Don't forget to squash the commits and then you should be good to go. 🙂