Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/ert/gui/plotting/ert_plots/__init__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from .cesp import CrossEnsembleStatisticsPlot
from .cross import CrossPlot
from .distribution import DistributionPlot
from .gaussian_kde import GaussianKDEPlot
from .histogram import HistogramPlot
Expand All @@ -8,6 +9,7 @@

__all__ = [
"CrossEnsembleStatisticsPlot",
"CrossPlot",
"DistributionPlot",
"GaussianKDEPlot",
"HistogramPlot",
Expand Down
192 changes: 192 additions & 0 deletions src/ert/gui/plotting/ert_plots/cross.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,192 @@
from __future__ import annotations

from typing import TYPE_CHECKING

import numpy as np
import pandas as pd

from ert.gui.plotting.plot_api import EnsembleObject, PlotApiKeyDefinition
from ert.gui.plotting.utils.plot_tools import ConditionalAxisFormatter, PlotTools
from ert.gui.utils import truncate_experiment_name

if TYPE_CHECKING:
import numpy.typing as npt
from matplotlib.axes import Axes
from matplotlib.figure import Figure

from ert.gui.plotting.utils import PlotConfig, PlotContext
from ert.gui.plotting.utils.plot_types import ObservationPlotLocations

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No clue where it is, but in General Options there is an "Observations" checkbox.
I think disabling/enabling it makes no sense for the crossplot.

Do we have a nice way of disabling it for Cross plots? Should be possible, as it is not available for Misfits.


class CrossPlot:
def __init__(self) -> None:
self.dimensionality = 2
self.requires_observations = True

@staticmethod
def plot(
figure: Figure,
plot_context: PlotContext,
ensemble_to_data_map: dict[EnsembleObject, pd.DataFrame],
observation_data: pd.DataFrame,
std_dev_images: dict[str, npt.NDArray[np.float32]],
obs_loc: ObservationPlotLocations | None,
key_def: PlotApiKeyDefinition | None = None,
) -> None:
plotCross(figure, plot_context, ensemble_to_data_map, observation_data)


def plotCross(
figure: Figure,
plot_context: PlotContext,
ensemble_to_data_map: dict[EnsembleObject, pd.DataFrame],
observation_data: pd.DataFrame,
) -> None:
config = plot_context.plotConfig()
axes = figure.add_subplot(111)

plot_context.x_axis = plot_context.VALUE_AXIS
plot_context.y_axis = plot_context.VALUE_AXIS
plot_context.deactivate_date_support()

if observation_data.empty:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

observation_data is also empty when observations exist, but no response keywords were added (so no responses for keyword exist).

Not sure what could be done with it, as currently there is no clear distinction what is wrong.
Maybe update error message to something like "No observations or responses are available for selected data type key" ?

axes.text(0.5, 0.5, "No observations available", ha="center", va="center")
axes.set_axis_off()
return

obs_by_key_index = _observations_by_key_index(observation_data)

all_obs: list[np.ndarray] = []
all_resp: list[np.ndarray] = []

for (ensemble, data), color_index in zip(
ensemble_to_data_map.items(),
plot_context.ensembles_color_indexes(),
strict=False,
):
config.set_current_color(color_index)

if data.empty:
continue

obs_values, resp_values = _match_obs_to_responses(data, obs_by_key_index)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

observation data is created via stacking data from all ensembles. What ensemble this observation data came from is lost.
Then we match this data per ensemble response.
So assuming that in ensemble A observations had keys [key1] and responses had keys [key2] and in ensemble B observations had keys [key2] and responses had keys [key2], combined observations would have keys [key1, key2], so not only ensemble B would match its key2 responses to observations, but ensemble A would match its key2 responses to ensemble B observations.

One ensemble selected. Ensemble is created such that only its first observation key matched response key, so there is just one datapoint:

Image

Adding second ensemble which has observations with keys fully matching responses of the first ensemble:

Image

Suddenly first ensemble got new points.

I would assume this is not desired behavior, though probably not affecting users too much as they more likely to compare stuff from the same experiment. (if this was previously discussed with Head of ERT and confirmed acceptable, there should be a comment about it somewhere as this behavior looks bug-ish).

I do not know if this is affecting any other plots, might be worth checking.

if obs_values.size == 0:
continue

label = (
f"{truncate_experiment_name(ensemble.experiment_name)} : {ensemble.name}"
)
_plot_cross(axes, config, obs_values, resp_values, label)

all_obs.append(obs_values)
all_resp.append(resp_values)

if all_obs:
_plot_identity_line(
axes, config, np.concatenate(all_obs), np.concatenate(all_resp)
)

axes.xaxis.set_major_formatter(ConditionalAxisFormatter())
axes.yaxis.set_major_formatter(ConditionalAxisFormatter())

if plot_context.log_scale:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do I set these?
I thought it is done via checkbox in plot controls and it is not available here?

axes.set_xscale("log")
axes.set_yscale("log")

PlotTools.finalize_plot(
plot_context,
figure,
axes,
default_x_label="Observation",
default_y_label="Response",
)


def _observations_by_key_index(observation_data: pd.DataFrame) -> pd.Series:
obs_values = pd.to_numeric(
observation_data.loc["OBS"].to_numpy(), errors="coerce"
).astype(np.float32)
raw_key_index = observation_data.loc["key_index"].to_numpy()
key_index = _to_matchable_key(raw_key_index)
series = pd.Series(obs_values, index=key_index)
return series[~series.index.duplicated(keep="first")].dropna()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When can index be duplicated?
Index by definition is supposed to be unique, so this line is suspicious.

I only see this happening because observation data is stacked together from several ensembles (that can in theory belong to different experiments).
But if in ensemble A for index 100 value is 10 and in ensemble B for index 100 value is 10000, it does not seem sensible that responses from B would be matched with observations from A because we kept first value.

One es_mda ensemble selected:

Image

Two ensembles selected:

Image

es_mda suddenly mutated into values from selected ensemble because only first values for index key were kept.



def _match_obs_to_responses(
ensemble_data: pd.DataFrame, obs_by_key_index: pd.Series
) -> tuple[np.ndarray, np.ndarray]:
responses = ensemble_data.copy()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we would still need to copy this if we manage to do something with the keys, so that there would be no need to exchange columns with _to_matchable_key

(yes, I struggle to understand why we need _to_matchable_key in the first place 😄 )

responses.columns = _to_matchable_key(responses.columns.to_numpy())

common_keys = obs_by_key_index.index.intersection(responses.columns)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everywhere we match observations and responses (in update's get_responses_and_observations and in EnsembleWidget), we now match with tolerance use_observation_locations_in_respective_responses because there is no guarantee that float values would always be exactly the same.
So do match calculations here require some tolerance-adjustments as well? Wouldn't small mismatch along the line throw all calculations off?

@achaikou achaikou Aug 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect this key_index data is matched upon is the one coming from plot_api's observations_for_key which in turn comes from dark_storage's get_observations_for_response

If so, then it is seismic_distance_expression - cumulative euclidean distance function that I introduced just so initial ensemble plots would display something. As you killed the ensemble plot, I think it should not be used anymore.

It creates good picture when everything matches, but worse when something does not.
Because distance is cumulative points-related distance, first key_index is always 0 (while the next ones differ depending on the actual east-north values).

But any points will have first distance zero, even if there are supposed to be no matches. But key_index = 0 forces one match.
So there always be one point present, even if I choose response data that does not at all match observation data.

Image

So is this cumulative distance as index key still good enough or is it too complex for our current needs? Would simple 1, 2, 3, 4... indexing of east-north pairs work better now (or on the opposite, worse, as match information gets totally lost)? Should it be key-index or is it a separate property now?


Some other reservoir data types where key_index is float would have similar problems if cross plot is enabled for them, so keep that in mind when designing too. For example for "RFT", key_index is tvd (though it should actually be "cell depth", because retrieved property seems to be "depth" and "tvd" and "depth" are two different things 🤔 ), but match key (which is used to match responses and observations) is well_connection_cell.
So need to figure out what key_index is used for now 😄

if len(common_keys) == 0:
return np.array([], dtype=np.float32), np.array([], dtype=np.float32)

obs_matched = obs_by_key_index.loc[common_keys].to_numpy()
resp_matched = (
responses[common_keys]
.apply(pd.to_numeric, errors="coerce")
.to_numpy()
.astype(np.float32)
)

n_realizations = resp_matched.shape[0]
obs_flat = np.tile(obs_matched, n_realizations)
resp_flat = resp_matched.reshape(-1)

mask = ~np.isnan(obs_flat) & ~np.isnan(resp_flat)
return obs_flat[mask], resp_flat[mask]


def _to_matchable_key(values: np.ndarray) -> np.ndarray:
numeric = pd.to_numeric(pd.Series(values), errors="coerce")
if numeric.notna().all():
return numeric.astype(np.float32).to_numpy()
return pd.Index(values).astype(str).to_numpy()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this function may return numeric key or string key?
What if observations return numeric key and responses return string key?
Can those be matched?
And why do we need both, numbers and strings? What is the danger?

(And generally this function seems purely technical, with no domain logic behind it. It makes me wonder if we really need this function).



def _plot_cross(
axes: Axes,
plot_config: PlotConfig,
obs_values: np.ndarray,
resp_values: np.ndarray,
label: str,
) -> None:
style = plot_config.distribution_style()

lines = axes.plot(
obs_values,
resp_values,
color=style.color,
alpha=style.alpha,
marker=style.marker or "o",
markersize=style.size,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Users didn't say anything, but I just wonder if we shouldn't reconsider size due to expected number of points:

Current:

Image

x 0.1:

Image

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good point, I agree

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

I don't think it's helpful in being able to distinguish the colors for the different ensemble, so I would only use it if we end up having a certain amount of datapoints that we need to plot. Open for discussions, let's document here what we agree on.

linestyle="",
label=label,
)

if lines:
plot_config.add_legend_item(label, lines[0])


def _plot_identity_line(
axes: Axes,
plot_config: PlotConfig,
obs_values: np.ndarray,
resp_values: np.ndarray,
) -> None:
lo = float(min(obs_values.min(), resp_values.min()))
hi = float(max(obs_values.max(), resp_values.max()))
if not np.isfinite(lo) or not np.isfinite(hi) or lo == hi:
return

lines = axes.plot(
[lo, hi],
[lo, hi],
color="red",
linestyle="-",
linewidth=1,
zorder=0,
)
if lines:
plot_config.add_legend_item("y = x", lines[0])
2 changes: 2 additions & 0 deletions src/ert/gui/plotting/plot_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ class EnsembleObject:
started_at: str
has_func_eval: bool = False
has_gradient: bool = False
size: int = 0

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should ensemble size even have a default value?
Pretty sure every valid ensemble would have its size set.



class PlotApiKeyDefinition(NamedTuple):
Expand Down Expand Up @@ -133,6 +134,7 @@ def get_all_ensembles(self) -> list[EnsembleObject]:
has_gradient=bool(
response_json["userdata"].get("has_gradient", False)
),
size=int(response_json.get("size", 0) or 0),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I struggle to understand why we need both 0 as default value if size is not present and or 0 as a value if this is an empty string.

From what I see, sent value is size=ensemble.ensemble_size , so it must be a valid int.
So why not a simple
size=int(response_json["size"])
?

)
)
except IndexError as exc:
Expand Down
30 changes: 29 additions & 1 deletion src/ert/gui/plotting/plot_window.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
from ert.gui.ertwidgets import CopyButton, showWaitCursorWhileWaiting
from ert.gui.plotting.utils.plot_maps import (
CROSS_ENSEMBLE_STATISTICS,
CROSSPLOT,
DISTRIBUTION,
ENSEMBLE,
ERT_PLOT_MAP,
Expand Down Expand Up @@ -70,6 +71,7 @@
StatisticsOptions,
)
from .widgets.plot_ensemble_selection_widget import EnsembleSelectionWidget
from .widgets.plot_realization_selection_widget import RealizationSelectionWidget
from .widgets.plot_widget import Plotter, PlotWidget

EVEREST_UPPER_BATCH_LIMIT = 20
Expand Down Expand Up @@ -298,6 +300,19 @@ def __init__(
expanded=True,
)

max_realizations = max((ens.size for ens in plot_case_objects), default=0)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably the safest from the developer's point of view (as whenever user clicks, number of ensembles stays the same), but I am not sure about users.

When you suggested it, I assumed you meant "choose max from selected ensembles", not from all of the loaded ones. So if one ensemble has 200 realizations and my current test ensemble that I am looking at has 10, all 200 are displayed.

Not sure what is the best option here, try to limit those or leave it as you suggest, as it shouldn't be a big problem for the users. 🤔

self._realization_selection_widget = RealizationSelectionWidget(
[str(i) for i in range(max_realizations)]
)
self._realization_selection_widget.realizationSelectionChanged.connect(
self.update_plot
)
self._realization_group = CollapsibleSection(
"Select realization",
create_group_layout([self._realization_selection_widget]),
expanded=True,
)

self._everest_controls_plot_options = EverestControlsPlotOptions(
self.update_plot
)
Expand All @@ -317,6 +332,7 @@ def __init__(
self._general_options.get_widget(),
self._everest_controls_plot_options.get_widget(),
self._everest_controls_group,
self._realization_group,
self._boxplot_options.get_widget(),
self._statistics_options.get_widget(),
]
Expand All @@ -327,6 +343,7 @@ def __init__(

self._everest_controls_group.setVisible(False)
self._everest_controls_plot_options.get_widget().setVisible(False)
self._realization_group.setVisible(False)
self._boxplot_options.get_widget().setVisible(False)
self._statistics_options.get_widget().setVisible(False)
self._data_type_keys_widget.selectDefault()
Expand Down Expand Up @@ -418,6 +435,7 @@ def update_plot(self, layer: int | None = None) -> None:
)
self._statistics_options.get_widget().setVisible(plot_widget.name == STATISTICS)
self._general_options.get_widget().setVisible(plot_widget.name != STD_DEV)
self._realization_group.setVisible(plot_widget.name == CROSSPLOT)

is_gradient_plot = plot_widget.name == EVEREST_GRADIENTS_PLOT
is_controls_plot = plot_widget.name == EVEREST_CONTROLS_PLOT
Expand Down Expand Up @@ -515,6 +533,15 @@ def fetch_data(
elif result is not None:
ensemble_to_data_map[ensemble] = result

if plot_widget.name == CROSSPLOT:
selected_realization = (
self._realization_selection_widget.get_selected_realization()
)
if selected_realization is not None:
for ensemble, data in list(ensemble_to_data_map.items()):
mask = data.index.astype(str) == selected_realization
ensemble_to_data_map[ensemble] = data.loc[mask]

log_scale_valid_values = True
if key_def.parameter is not None and key_def.parameter.type == "gen_kw":
for data in ensemble_to_data_map.values():
Expand Down Expand Up @@ -775,7 +802,8 @@ def keySelected(self) -> None:
if widget._plotter.dimensionality == key_def.dimensionality
and (key_def.observations or not widget._plotter.requires_observations)
and not is_everest_specific_widget
and (not is_observed_seismic or widget.name == MISFITS)
and (not is_observed_seismic or widget.name in {MISFITS, CROSSPLOT})
and (widget.name != CROSSPLOT or is_observed_seismic)
]

def everest_data_origin_check(origin: list[str]) -> bool:
Expand Down
3 changes: 3 additions & 0 deletions src/ert/gui/plotting/utils/plot_maps.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from ert.gui.plotting.ert_plots import (
CrossEnsembleStatisticsPlot,
CrossPlot,
DistributionPlot,
GaussianKDEPlot,
HistogramPlot,
Expand All @@ -27,6 +28,7 @@
STATISTICS = "Statistics"
STD_DEV = "Std dev"
MISFITS = "Misfits"
CROSSPLOT = "Cross plot"
EVEREST_CONTROLS_PLOT = "Controls"
EVEREST_GRADIENTS_PLOT = "Gradient"
EVEREST_OBJECTIVE_FUNCTION_PLOT = "Objective function"
Expand All @@ -41,6 +43,7 @@
DISTRIBUTION: DistributionPlot,
CROSS_ENSEMBLE_STATISTICS: CrossEnsembleStatisticsPlot,
STD_DEV: StdDevPlot,
CROSSPLOT: CrossPlot,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this tab shouldn't be placed right after the misfits?

I think all the plots to the right of misfit activate for parameters comparison, misfit being the last one two actually compare ensembles.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So you would want to place the Crossplot tab to the left of Misfits?

}
EVEREST_PLOT_MAP: dict[str, Callable[[], Plotter]] = {
EVEREST_BATCH_OBJECTIVE_FUNCTION_PLOT: EverestBatchObjectiveFunctionPlot,
Expand Down
51 changes: 51 additions & 0 deletions src/ert/gui/plotting/widgets/plot_realization_selection_widget.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
from PyQt6.QtCore import pyqtSignal as Signal
from PyQt6.QtWidgets import (
QAbstractItemView,
QListWidget,
QListWidgetItem,
QVBoxLayout,
QWidget,
)


class RealizationSelectionWidget(QWidget):
realizationSelectionChanged = Signal()

def __init__(
self,
realizations: list[str],
) -> None:
super().__init__()
self._realizations_list = QListWidget()
self._realizations_list.setSelectionMode(
QAbstractItemView.SelectionMode.SingleSelection
)
self._realizations_list.itemSelectionChanged.connect(self._onSelectionChanged)

layout = QVBoxLayout()
layout.addWidget(self._realizations_list)
layout.setContentsMargins(0, 0, 0, 0)
self.setLayout(layout)

self.set_realizations(realizations)

def set_realizations(self, realizations: list[str]) -> None:
self._realizations_list.clear()
for realization in realizations:
item = QListWidgetItem(realization)
self._realizations_list.addItem(item)

if self._realizations_list.count() > 0:
first_item = self._realizations_list.item(0)
if first_item is not None:
first_item.setSelected(True)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this line needed?
Or will
self._realizations_list.setCurrentItem(first_item)
suffice?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is 4 marked lines here, can you elaborate which one you specifically mean?

self._realizations_list.setCurrentItem(first_item)

def get_selected_realization(self) -> str | None:
selected_realization = self._realizations_list.currentItem()
if selected_realization:
return selected_realization.text()
return None

def _onSelectionChanged(self) -> None:
self.realizationSelectionChanged.emit()
1 change: 1 addition & 0 deletions tests/ert/ui_tests/gui/test_main_window.py
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,7 @@ def test_that_the_plot_window_contains_the_expected_elements(
"Statistics",
"Std dev",
"Misfits",
"Cross plot",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding commit name "fixup! Add seismic cross plot" in which this change is added:

I am not sure if you plan to squash and merge (then it doesn't matter), or rebase and merge (then this fixup change probably should refer to a different commit, as this is a test file).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"fixup! Add seismic cross plot" is a fixup for this specific commit, because locally I ran "just rapid-tests" which passed, but then discovered that by adding the cross plot there was a gui test failing, there for it is a fixup to the first commit.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And yes, I'll squash the fixups into the commits which they are a fixup for.

}

model = data_keys.model()
Expand Down
Loading
Loading