equinor / ert

ERT - Ensemble based Reservoir Tool - is designed for running ensembles of dynamical models such as reservoir models, in order to do sensitivity analysis and data assimilation. ERT supports data assimilation using the Ensemble Smoother (ES), Ensemble Smoother with Multiple Data Assimilation (ES-MDA) and Iterative Ensemble Smoother (IES).
https://ert.readthedocs.io/en/latest/
GNU General Public License v3.0
98 stars 104 forks source link

More separation of concerns between GUI and Storage #8128

Open yngve-sk opened 3 weeks ago

yngve-sk commented 3 weeks ago

Right now, some storage logic is done "directly" in the GUI, it should ideally be more separate, and data dissection like this should be done elsewhere so that GUI is not subject to changes in the way storage represents its data internally.

Example: things similar to this (from storage_info_widget.py:

        # check if the response is empty
        if bool(response_ds.dims):
            if response_name == "summary":
                response_ds = response_ds.sel(name=str(observation_ds.name.data[0]))

            if "time" in observation_ds.coords:
                observation_ds = observation_ds.sel(time=observation_label)
                response_ds = response_ds.sel(time=observation_label)
            elif "index" in observation_ds.coords:
                observation_ds = observation_ds.sel(index=int(observation_label))
                response_ds = response_ds.drop(["index"]).sel(
                    index=int(observation_label)
                )

should be done on the storage side.

Discussion point wrt where to put the logic and how: I will list some options:

  1. Create a _ert/storage/*.py API with classes that inherit from our public API LocalStorage/LocalEnsemble/LocalExperiment and also implement some more functionality needed for specifics within the GUI. Then we don't have to care that it is a public interface and we'll be more free to implement just what ert needs.
  2. Put it all in public API
  3. Put all GUI/Storage interaction logic into one thing, and this thing will be the interface between the two. Would expose the "minimal" functionality needed by GUI, and abstract over some calls into storage. We could realize this by creating a separate small (either under ert or _ert for privacy) class that holds some Storage objects, and make the GUI always go through this and never storage directly. Adds one more layer of indirection but gives a cleaner layer between the two. This "thing" could be thought of as the API into the backend that exposes controls/data to the frontend.
  4. Add more/new/refactored endpoints to dark storage for all the things. (Some things might make sense to just put in the public API, suggestions above are for the things that might be more specific to storage)

Personal preference: 3>1

oyvindeide commented 3 weeks ago

Not sure I understand point 3, do you want to move Storage/Experiment there, or just have utility functions for interacting? With regards to the api interaction in the storage widget, there was an enpoint for calculating exactly this that was added in the current refactor?

In general I agree with you though, that we should be more careful regarding the interaction with storage.

yngve-sk commented 3 weeks ago

I tried to clarify the text on nr.3 some, it basically means just having utility functions for interacting, gathered in one place. To be thought of as an API as if it was a web API, as minimal as possible.

WRT api interaction with the storage widget, I think we could use the get_observations_and_responses to get the data needed for rendering observations/responses with errors bars etc, but there are also some calls to get_realization_state etc, and we do need to transfer this data somehow, but at least putting it in one place would let us see the extent of the interface between the storage and the GUI is more easily.