Skip to content

Commit

Permalink
Improve handing of invalid experiments in GUI
Browse files Browse the repository at this point in the history
This commit improves the handling of invalid experiments in the gui,
in the case of missing responses.json file. The handling of the missing
file should in the future be extended to also handle the other experiment
files in a similar manner (index.json, metadata.json, and parameters.json)

This commit:
* Makes storage reload and re-validate when changing between ert modes
  (manage experient, run experiment, and plot tool)
* Disables ensemble with invalid experiments from ensemble_selectors
  (error is shown as tooltip on hover)
* Filters out invalid experiments from dark storage, so plotter won't
  attempt to plot them.
* Reloads and re-validates storage on end of experiment, so ert won't
  crash if responses.json is deleted mid-run.
  • Loading branch information
jonathan-eq committed Jan 23, 2025
1 parent 9ab687a commit 033426e
Show file tree
Hide file tree
Showing 17 changed files with 230 additions and 26 deletions.
1 change: 1 addition & 0 deletions src/ert/dark_storage/endpoints/experiments.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ def get_experiments(
userdata={},
)
for experiment in storage.experiments
if experiment.is_valid()
]


Expand Down
3 changes: 1 addition & 2 deletions src/ert/dark_storage/endpoints/records.py
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,7 @@ def get_ensemble_responses(

response_names_with_observations = set()
observations = ensemble.experiment.observations

if len(ensemble.has_data()) == 0:
if not ensemble.experiment.is_valid() or len(ensemble.has_data()) == 0:
return {}

for (
Expand Down
13 changes: 13 additions & 0 deletions src/ert/gui/ertnotifier.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,3 +63,16 @@ def set_current_ensemble(self, ensemble: Ensemble | None = None) -> None:
@Slot(bool)
def set_is_simulation_running(self, is_running: bool) -> None:
self._is_simulation_running = is_running
self.refresh()

def refresh(self) -> None:
if self._storage is None:
return
self._storage.refresh()
self.storage_changed.emit(self._storage)

def revalidate_storage(self) -> None:
if self._storage is None:
return
if self._storage.check_if_experiment_validity_changed():
self.refresh()
27 changes: 25 additions & 2 deletions src/ert/gui/ertwidgets/ensembleselector.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
from __future__ import annotations

from collections.abc import Iterable
from typing import TYPE_CHECKING
from typing import TYPE_CHECKING, cast

from qtpy.QtCore import Qt, Signal
from qtpy.QtGui import QStandardItemModel
from qtpy.QtWidgets import QComboBox

from ert.gui.ertnotifier import ErtNotifier
Expand All @@ -22,6 +23,7 @@ def __init__(
update_ert: bool = True,
show_only_undefined: bool = False,
show_only_no_children: bool = False,
show_only_with_valid_experiment: bool = False,
):
super().__init__()
self.notifier = notifier
Expand All @@ -36,6 +38,7 @@ def __init__(
# if the ensemble has not been used in an update, as that would
# invalidate the result
self._show_only_no_children = show_only_no_children
self._show_only_with_valid_experiment = show_only_with_valid_experiment
self.setSizeAdjustPolicy(QComboBox.AdjustToContents)

self.setEnabled(False)
Expand Down Expand Up @@ -68,9 +71,26 @@ def populate(self) -> None:
self.setEnabled(True)

for ensemble in self._ensemble_list():
model = cast(QStandardItemModel, self.model())
self.addItem(
f"{ensemble.experiment.name} : {ensemble.name}", userData=ensemble
)
if (
self._show_only_with_valid_experiment
and not ensemble.experiment.is_valid()
):
index = self.count() - 1
model_item = model.item(index)
assert model_item is not None
new_flags = (
model_item.flags()
& ~Qt.ItemFlags(Qt.ItemFlag.ItemIsEnabled)
& ~Qt.ItemFlags(Qt.ItemFlag.ItemIsSelectable)
)
model_item.setFlags(new_flags)
self.setItemData(
index, "This ensemble is invalid", Qt.ItemDataRole.ToolTipRole
)

current_index = self.findData(
self.notifier.current_ensemble, Qt.ItemDataRole.UserRole
Expand All @@ -94,7 +114,10 @@ def _ensemble_list(self) -> Iterable[Ensemble]:
)
else:
ensembles = self.notifier.storage.ensembles
ensemble_list = list(ensembles)
if self._show_only_with_valid_experiment:
ensemble_list = [ens for ens in ensembles if ens.experiment.is_valid()]
else:
ensemble_list = list(ensembles)
if self._show_only_no_children:
parents = [
ens.parent for ens in self.notifier.storage.ensembles if ens.parent
Expand Down
1 change: 1 addition & 0 deletions src/ert/gui/main_window.py
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ def right_clicked(self) -> None:
def select_central_widget(self) -> None:
actor = self.sender()
if actor:
self.notifier.revalidate_storage()
index_name = actor.property("index")

for widget in self.central_panels_map.values():
Expand Down
11 changes: 8 additions & 3 deletions src/ert/gui/model/real_list.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from typing import overload
from typing import cast, overload

from qtpy.QtCore import (
QAbstractItemModel,
Expand All @@ -9,7 +9,12 @@
)
from typing_extensions import override

from ert.gui.model.snapshot import IsEnsembleRole, IsRealizationRole, NodeRole
from ert.gui.model.snapshot import (
IsEnsembleRole,
IsRealizationRole,
NodeRole,
SnapshotModel,
)


class RealListModel(QAbstractProxyModel):
Expand Down Expand Up @@ -67,7 +72,7 @@ def columnCount(self, parent: QModelIndex | None = None) -> int:
def rowCount(self, parent: QModelIndex | None = None) -> int:
parent = parent if parent else QModelIndex()
if not parent.isValid():
source_model = self.sourceModel()
source_model = cast(SnapshotModel, self.sourceModel())
assert source_model is not None
iter_index = source_model.index(self._iter, 0, QModelIndex())
if iter_index.isValid():
Expand Down
4 changes: 3 additions & 1 deletion src/ert/gui/simulation/evaluate_ensemble_panel.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,9 @@ def __init__(self, ensemble_size: int, run_path: str, notifier: ErtNotifier):
lab.setWordWrap(True)
lab.setAlignment(QtCore.Qt.AlignmentFlag.AlignLeft)
layout.addRow(lab)
self._ensemble_selector = EnsembleSelector(notifier, show_only_no_children=True)
self._ensemble_selector = EnsembleSelector(
notifier, show_only_no_children=True, show_only_with_valid_experiment=True
)
layout.addRow("Ensemble:", self._ensemble_selector)
runpath_label = CopyableLabel(text=run_path)
layout.addRow("Runpath:", runpath_label)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,11 @@ def _add_initialize_from_scratch_tab(self) -> None:

ensemble_layout = QHBoxLayout()
ensemble_label = QLabel("Target ensemble:")
ensemble_selector = EnsembleSelector(self.notifier, show_only_undefined=True)
ensemble_selector = EnsembleSelector(
self.notifier,
show_only_undefined=True,
show_only_with_valid_experiment=True,
)
ensemble_selector.setMinimumWidth(300)
ensemble_layout.addWidget(ensemble_label)
ensemble_layout.addWidget(ensemble_selector)
Expand Down
2 changes: 2 additions & 0 deletions src/ert/gui/tools/manage_experiments/storage_info_widget.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ def __init__(self) -> None:

@Slot(Experiment)
def setExperiment(self, experiment: Experiment) -> None:
if not experiment.is_valid():
return
self._experiment = experiment

self._name_label.setText(f"Name: {experiment.name!s}")
Expand Down
40 changes: 31 additions & 9 deletions src/ert/gui/tools/manage_experiments/storage_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ def row(self) -> int:
def data(self, index: QModelIndex, role: Qt.ItemDataRole) -> Any:
if not index.isValid():
return None

col = index.column()
if role == Qt.ItemDataRole.DisplayRole:
if col == _Column.NAME:
Expand All @@ -85,15 +84,16 @@ def data(self, index: QModelIndex, role: Qt.ItemDataRole) -> Any:
elif role == Qt.ItemDataRole.ToolTipRole:
if col == _Column.TIME:
return str(self._start_time)

return None


class ExperimentModel:
def __init__(self, experiment: Experiment, parent: Any):
class ExperimentModel(QAbstractItemModel):
def __init__(self, experiment: Experiment, parent: "StorageModel"):
self._parent = parent
self._id = experiment.id
self._name = experiment.name
self._is_valid = experiment.is_valid()
self._error = experiment.error_message
self._experiment_type = experiment.metadata.get("ensemble_type")
self._children: list[EnsembleModel] = []

Expand All @@ -105,9 +105,7 @@ def row(self) -> int:
return self._parent._children.index(self)
return 0

def data(
self, index: QModelIndex, role: Qt.ItemDataRole = Qt.ItemDataRole.DisplayRole
) -> Any:
def data(self, index: QModelIndex, role: int = Qt.ItemDataRole.DisplayRole) -> Any:
if not index.isValid():
return None

Expand All @@ -128,7 +126,9 @@ def data(
qapp = QApplication.instance()
assert isinstance(qapp, QApplication)
return qapp.palette().mid()

elif role == Qt.ItemDataRole.ToolTipRole:
if self._error:
return self._error
return None


Expand Down Expand Up @@ -211,9 +211,31 @@ def headerData(
def data(self, index: QModelIndex, role: int = Qt.ItemDataRole.DisplayRole) -> Any:
if not index.isValid():
return None

return index.internalPointer().data(index, role)

@override
def flags(self, index: QModelIndex) -> Qt.ItemFlags:
default_flags = super().flags(index)
if not index.isValid():
return default_flags
item = index.internalPointer()
if isinstance(item, ExperimentModel) and not item._is_valid:
new_flags = default_flags & ~Qt.ItemFlags(Qt.ItemFlag.ItemIsEnabled)
return Qt.ItemFlags(new_flags)
return default_flags

@override
def hasChildren(self, parent: QModelIndex | None = None) -> bool:
if parent is None or not parent.isValid():
return True

flags = self.flags(parent)
# hide children if disabled
if not (flags & Qt.ItemFlag.ItemIsEnabled):
return False

return super().hasChildren(parent)

@override
def index(
self, row: int, column: int, parent: QModelIndex | None = None
Expand Down
4 changes: 2 additions & 2 deletions src/ert/gui/tools/manage_experiments/storage_widget.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,13 +133,13 @@ def __init__(
def _currentChanged(self, selected: QModelIndex, previous: QModelIndex) -> None:
idx = self._tree_view.model().mapToSource(selected) # type: ignore
cls = idx.internalPointer()

if isinstance(cls, EnsembleModel):
ensemble = self._notifier.storage.get_ensemble(cls._id)
self.onSelectEnsemble.emit(ensemble)
elif isinstance(cls, ExperimentModel):
experiment = self._notifier.storage.get_experiment(cls._id)
self.onSelectExperiment.emit(experiment)
if experiment.is_valid():
self.onSelectExperiment.emit(experiment)
elif isinstance(cls, RealizationModel):
ensemble = self._notifier.storage.get_ensemble(cls.ensemble_id)
self.onSelectRealization.emit(ensemble, cls.realization)
Expand Down
1 change: 0 additions & 1 deletion src/ert/gui/tools/plot/plot_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,6 @@ def all_data_type_keys(self) -> list[PlotApiKeyDefinition]:
f"/experiments/{experiment['id']}/ensembles", timeout=self._timeout
)
self._check_response(response)

for ensemble in response.json():
response = client.get(
f"/ensembles/{ensemble['id']}/responses", timeout=self._timeout
Expand Down
6 changes: 6 additions & 0 deletions src/ert/storage/local_ensemble.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ def create_realization_dir(realization: int) -> Path:
return self._path / f"realization-{realization}"

self._realization_dir = create_realization_dir
self.has_valid_experiment: bool | None = None

@classmethod
def create(
Expand Down Expand Up @@ -406,6 +407,11 @@ def get_ensemble_state(self) -> list[set[RealizationStorageState]]:
states : list of RealizationStorageState
list of realization states.
"""
if not self.experiment.is_valid():
logger.warning(
f"Could not get ensemble state for ensemble ({self.id}) due to invalid experiment ({self.experiment_id}): {self.experiment.error_message}"
)
return []

response_configs = self.experiment.response_configuration

Expand Down
20 changes: 20 additions & 0 deletions src/ert/storage/local_experiment.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,26 @@ def __init__(
self._index = _Index.model_validate_json(
(path / "index.json").read_text(encoding="utf-8")
)
self._validate_files()

def _validate_files(self) -> None:
self.valid_parameters = (self._path / self._parameter_file).exists()
self.valid_responses = (self._path / self._responses_file).exists()
self.valid_metadata = (self._path / self._metadata_file).exists()

def is_valid(self) -> bool:
return self.valid_parameters and self.valid_responses and self.valid_metadata

@property
def error_message(self) -> str:
errors = []
if not self.valid_parameters:
errors.append("Parameter file is missing")
if not self.valid_responses:
errors.append("Responses file is missing")
if not self.valid_metadata:
errors.append("Metadata file is missing")
return "\n".join(errors)

@classmethod
def create(
Expand Down
27 changes: 22 additions & 5 deletions src/ert/storage/local_storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,19 @@ def refresh(self) -> None:
for ens in self._ensembles.values():
ens.refresh_ensemble_state()

def check_if_experiment_validity_changed(self) -> bool:
"""
Checks if any of the experiments have changed validity since we last checked.
The validity is determined by the presence of the required files (responses.json, index.json, metadata.json, and parameters.json.)
"""
for exp in self._experiments.values():
exp_was_valid = exp.is_valid()
exp._validate_files()
if exp.is_valid() != exp_was_valid:
return True
return False

def get_experiment(self, uuid: UUID) -> LocalExperiment:
"""
Retrieves an experiment by UUID.
Expand Down Expand Up @@ -226,11 +239,15 @@ def _load_ensembles(self) -> dict[UUID, LocalEnsemble]:
}

def _load_experiments(self) -> dict[UUID, LocalExperiment]:
experiment_ids = {ens.experiment_id for ens in self._ensembles.values()}
return {
exp_id: LocalExperiment(self, self._experiment_path(exp_id), self.mode)
for exp_id in experiment_ids
}
experiments = {}
for ens in self._ensembles.values():
experiment = LocalExperiment(
self, self._experiment_path(ens.experiment_id), self.mode
)
ens.has_valid_experiment = experiment.is_valid()
experiments[ens.experiment_id] = experiment

return experiments

def _ensemble_path(self, ensemble_id: UUID) -> Path:
return self.path / self.ENSEMBLES_PATH / str(ensemble_id)
Expand Down
7 changes: 7 additions & 0 deletions tests/ert/ui_tests/gui/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,13 @@ def wait_for_child(gui, qtbot: QtBot, typ: type[V], timeout=5000, **kwargs) -> V
return get_child(gui, typ, **kwargs)


def wait_for_children(
gui, qtbot: QtBot, typ: type[V], timeout=5000, **kwargs
) -> list[V]:
qtbot.waitUntil(lambda: gui.findChildren(typ) is not None, timeout=timeout)
return get_children(gui, typ, **kwargs)


def get_child(gui: QWidget, typ: type[V], *args, **kwargs) -> V:
child = gui.findChild(typ, *args, **kwargs)
assert isinstance(child, typ)
Expand Down
Loading

0 comments on commit 033426e

Please sign in to comment.