Combine datasets & refactor observations to response configs #7563

yngve-sk · 2024-04-04T11:46:35Z

squash of #7495

Requires change in semeio: equinor/semeio#609

oyvindeide

Still have a bit to go through, so some comments as I go along

src/ert/config/observations.py

src/ert/dark_storage/common.py

src/ert/shared/share/ert/workflows/jobs/internal-gui/scripts/gen_data_rft_export.py

src/ert/dark_storage/endpoints/records.py

src/ert/storage/local_ensemble.py

tests/integration_tests/analysis/test_es_update.py

tests/unit_tests/storage/migration/test_version_3.py

src/ert/storage/local_storage.py

oyvindeide · 2024-05-21T10:54:29Z

tests/performance_tests/test_memory_usage.py

@@ -39,8 +39,8 @@ def poly_template(monkeypatch):
    yield folder


-@pytest.mark.flaky(reruns=5)
-@pytest.mark.limit_memory("130 MB")
+# @pytest.mark.flaky(reruns=5)


Does not need flaky marker anymore?

Seems not AFAIK, at least it hasn't been failing with the marker commented out

Then it should be removed and not commented out

tests/performance_tests/test_memory_usage.py

eivindjahren · 2024-05-24T10:49:26Z

closes #5065

oyvindeide

LGTM! Really good job, thanks for your patience in a long review! I have some minor comments left, but just look at them and force push.

oyvindeide · 2024-05-27T13:48:35Z

src/ert/gui/ertwidgets/storage_info_widget.py

@@ -191,7 +191,6 @@ def _currentItemChanged(
        response_name = observation_ds.attrs["response"]
        response_ds = self._ensemble.load_responses(
            response_name,
-            tuple(self._ensemble.get_realization_list_with_responses()),


Why was this removed?

Passing in None implicitly requests all realizations, so this was a bit redundant

oyvindeide · 2024-05-27T13:49:51Z

src/ert/run_models/base_run_model.py

+        run_context.ensemble.unify_parameters()
+        run_context.ensemble.unify_responses()


Should these be in self.run_ensemble_evaluator instead?

Seems to work so yes

oyvindeide · 2024-05-27T13:51:17Z

src/ert/storage/local_storage.py

@@ -272,6 +272,10 @@ def close(self) -> None:
        the storage.
        """

+        for ens in self._ensembles.values():
+            ens.unify_responses()


Need to check that we have write permission here

oyvindeide · 2024-05-27T13:51:32Z

src/ert/storage/local_storage.py

                migrations = list(enumerate([to2, to3, to4, to5, to6], start=1))
+                print(f"Found storage with version: {version} at: {self.path}")


Remove print

oyvindeide · 2024-05-27T13:52:00Z

src/ert/storage/migration/block_fs.py

@@ -574,3 +582,5 @@ def _migrate_gen_kw(
            }
        )
        ensemble.save_parameters(block.name, block.realization_index, dataset)
+
+    ensemble.unify_parameters()


Call this once at the end instead?

oyvindeide · 2024-05-27T13:53:16Z

tests/integration_tests/test_storage_migration.py

+@pytest.mark.parametrize(
+    "ert_version",
+    [
+        "8.4.9",


A few new were just added, should also include them

oyvindeide · 2024-05-27T13:53:45Z

tests/performance_tests/test_memory_usage.py

@@ -39,8 +39,8 @@ def poly_template(monkeypatch):
    yield folder


-@pytest.mark.flaky(reruns=5)
-@pytest.mark.limit_memory("130 MB")
+# @pytest.mark.flaky(reruns=5)


Then it should be removed and not commented out

oyvindeide · 2024-05-27T13:57:09Z

tests/unit_tests/scenarios/test_summary_response.py

-        [datetime(2014, 9, 9)],
-        [datetime(2014, 9, 9)],
-        [datetime(2017, 9, 9)],
+        [datetime(2014, 1, 22)],


This changed because of the ordering of keys?

Not sure, it seems to work with the old times also, so I'm just reverting it back to what it was

oyvindeide · 2024-05-27T13:57:24Z

tests/unit_tests/storage/migration/test_block_fs_simple_case.py

@@ -95,6 +97,7 @@ def test_migrate_case(data, storage, enspath):
        bf.migrate_case(storage, enspath / "default", stack)

    ensemble = storage.get_ensemble_by_name("default")
+    ensemble.unify_parameters()


Technically these are not needed now?

oyvindeide reviewed Apr 4, 2024

View reviewed changes

yngve-sk force-pushed the combine-ds-obs-squash branch 7 times, most recently from 2502dbf to 5519238 Compare April 5, 2024 11:47

yngve-sk mentioned this pull request Apr 5, 2024

Adapt to combined observations in ert equinor/semeio#609

Merged

yngve-sk force-pushed the combine-ds-obs-squash branch 21 times, most recently from 5225777 to 01d711c Compare April 9, 2024 08:24

yngve-sk requested a review from oyvindeide May 8, 2024 09:34

oyvindeide reviewed May 8, 2024

View reviewed changes

tests/unit_tests/storage/migration/test_version_3.py Outdated Show resolved Hide resolved

oyvindeide reviewed May 8, 2024

View reviewed changes

src/ert/storage/local_storage.py Show resolved Hide resolved

oyvindeide reviewed May 8, 2024

View reviewed changes

src/ert/storage/local_storage.py Outdated Show resolved Hide resolved

yngve-sk force-pushed the combine-ds-obs-squash branch 5 times, most recently from d909193 to df528bf Compare May 21, 2024 06:54

oyvindeide reviewed May 21, 2024

View reviewed changes

tests/performance_tests/test_memory_usage.py Show resolved Hide resolved

yngve-sk force-pushed the combine-ds-obs-squash branch 3 times, most recently from 3125a80 to 01b4537 Compare May 24, 2024 07:49

yngve-sk requested a review from oyvindeide May 24, 2024 08:24

eivindjahren closed this May 24, 2024

eivindjahren reopened this May 24, 2024

yngve-sk force-pushed the combine-ds-obs-squash branch 5 times, most recently from 052226f to 45c2569 Compare May 27, 2024 12:50

oyvindeide approved these changes May 27, 2024

View reviewed changes

yngve-sk force-pushed the combine-ds-obs-squash branch from b2a7932 to e876662 Compare May 28, 2024 06:11

Yngve S. Kristiansen added 2 commits May 28, 2024 08:31

Move observation parsing to configs

f9427e1

Combine obs & ds

dd60aeb

yngve-sk force-pushed the combine-ds-obs-squash branch from e876662 to dd60aeb Compare May 28, 2024 06:32

yngve-sk merged commit a5273b5 into equinor:main May 28, 2024
36 of 38 checks passed

eivindjahren mentioned this pull request Jun 27, 2024

Implement LocalEnsemble finalize function, which combines all realisations into one dataset. #5527

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Combine datasets & refactor observations to response configs #7563

Combine datasets & refactor observations to response configs #7563

yngve-sk commented Apr 4, 2024 •

edited

Loading

oyvindeide left a comment

oyvindeide May 21, 2024

yngve-sk May 21, 2024

oyvindeide May 27, 2024

eivindjahren commented May 24, 2024

oyvindeide left a comment

oyvindeide May 27, 2024

yngve-sk May 27, 2024

oyvindeide May 27, 2024

yngve-sk May 28, 2024

oyvindeide May 27, 2024

oyvindeide May 27, 2024

oyvindeide May 27, 2024

oyvindeide May 27, 2024

oyvindeide May 27, 2024

oyvindeide May 27, 2024

yngve-sk May 28, 2024

oyvindeide May 27, 2024

		run_context.ensemble.unify_parameters()
		run_context.ensemble.unify_responses()

		migrations = list(enumerate([to2, to3, to4, to5, to6], start=1))
		print(f"Found storage with version: {version} at: {self.path}")

Combine datasets & refactor observations to response configs #7563

Combine datasets & refactor observations to response configs #7563

Conversation

yngve-sk commented Apr 4, 2024 • edited Loading

oyvindeide left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eivindjahren commented May 24, 2024

oyvindeide left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yngve-sk commented Apr 4, 2024 •

edited

Loading