Skip to content

Commit 2df79e8

Browse files
SimonHeybrockclaude
andcommitted
Add overlay unit validation at both intra-layer and cross-layer sites
HoloViews silently renders overlaid elements against a single shared axis using the first element's units, producing incorrect plots when sources have mismatched units. This adds validation at two sites: Site 1 (Plotter.compute): validates that all DataArrays in an overlay share the same dimension names, coordinate units, and value units. Errors are caught by the existing try/except and displayed as text. Site 2 (_get_session_composed_plot): validates cross-layer unit compatibility before composing DynamicMaps. Each Plotter stores an OverlayUnitKey after compute(), which captures kdim and vdim unit metadata. ROI readback/request plotters provide kdim units from their coordinate metadata but skip vdim validation. Static plotters are naturally exempt (no Plotter instance). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 211465f commit 2df79e8

7 files changed

Lines changed: 470 additions & 2 deletions

File tree

src/ess/livedata/dashboard/plots.py

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,121 @@ def _compute_time_info(data: dict[str, sc.DataArray]) -> str | None:
214214
return f'{end_str} (Lag: {lag_s:.1f}s)'
215215

216216

217+
@dataclass(frozen=True)
218+
class OverlayUnitKey:
219+
"""Unit signature for cross-layer overlay validation.
220+
221+
Captures the units of all plotting dimensions and (optionally) the value
222+
dimension. Two keys are compatible when every shared kdim label has the
223+
same unit string and, if both keys carry a vdim unit, those match too.
224+
"""
225+
226+
kdim_units: tuple[tuple[str, str | None], ...] = ()
227+
vdim_unit: str | None = None
228+
229+
@classmethod
230+
def from_data_array(
231+
cls,
232+
da: sc.DataArray,
233+
*,
234+
include_vdim: bool = True,
235+
) -> OverlayUnitKey:
236+
"""Build a key from a scipp DataArray using its dimension coordinates."""
237+
kdim_units = tuple(
238+
sorted(
239+
(dim, _unit_str(da.coords[dim].unit))
240+
for dim in da.dims
241+
if dim in da.coords
242+
)
243+
)
244+
vdim_unit = _unit_str(da.unit) if include_vdim else None
245+
return cls(kdim_units=kdim_units, vdim_unit=vdim_unit)
246+
247+
248+
def _unit_str(unit: sc.Unit | None) -> str | None:
249+
"""Convert a scipp unit to its string representation, or None."""
250+
return str(unit) if unit is not None else None
251+
252+
253+
def _validate_overlay_units(data: dict[ResultKey, sc.DataArray]) -> None:
254+
"""Validate that all DataArrays in an overlay share compatible units.
255+
256+
Raises
257+
------
258+
ValueError
259+
If dimension names, coordinate units, or value units do not match.
260+
"""
261+
if len(data) < 2:
262+
return
263+
264+
items = iter(data.items())
265+
ref_key, ref = next(items)
266+
ref_source = ref_key.job_id.source_name
267+
268+
for key, da in items:
269+
source = key.job_id.source_name
270+
if ref.dims != da.dims:
271+
raise ValueError(
272+
f"Cannot overlay '{ref_source}' and '{source}': "
273+
f"dimension mismatch ({ref.dims} vs {da.dims})"
274+
)
275+
for dim in ref.dims:
276+
if dim in ref.coords and dim in da.coords:
277+
u_ref = ref.coords[dim].unit
278+
u_da = da.coords[dim].unit
279+
if u_ref != u_da:
280+
raise ValueError(
281+
f"Cannot overlay '{ref_source}' and '{source}': "
282+
f"unit mismatch for coordinate '{dim}' "
283+
f"({u_ref} vs {u_da})"
284+
)
285+
if ref.unit != da.unit:
286+
raise ValueError(
287+
f"Cannot overlay '{ref_source}' and '{source}': "
288+
f"value unit mismatch ({ref.unit} vs {da.unit})"
289+
)
290+
291+
292+
def validate_cross_layer_units(
293+
keys: list[tuple[str, OverlayUnitKey]],
294+
) -> str | None:
295+
"""Check overlay unit keys from multiple layers for compatibility.
296+
297+
Parameters
298+
----------
299+
keys:
300+
Pairs of (layer label, unit key) for each data-carrying layer.
301+
302+
Returns
303+
-------
304+
:
305+
An error message if units are incompatible, or None if compatible.
306+
"""
307+
if len(keys) < 2:
308+
return None
309+
310+
ref_label, ref_key = keys[0]
311+
ref_kdims = dict(ref_key.kdim_units)
312+
313+
for label, key in keys[1:]:
314+
other_kdims = dict(key.kdim_units)
315+
for dim_label, ref_unit in ref_kdims.items():
316+
if dim_label in other_kdims and ref_unit != other_kdims[dim_label]:
317+
return (
318+
f"Cannot overlay layers '{ref_label}' and '{label}': "
319+
f"unit mismatch for coordinate '{dim_label}' "
320+
f"({ref_unit} vs {other_kdims[dim_label]})"
321+
)
322+
if ref_key.vdim_unit is not None and key.vdim_unit is not None:
323+
if ref_key.vdim_unit != key.vdim_unit:
324+
return (
325+
f"Cannot overlay layers '{ref_label}' and '{label}': "
326+
f"value unit mismatch "
327+
f"({ref_key.vdim_unit} vs {key.vdim_unit})"
328+
)
329+
return None
330+
331+
217332
class Plotter:
218333
"""
219334
Base class for plots that support autoscaling.
@@ -245,6 +360,7 @@ def __init__(
245360
"""
246361
self._normalize_to_rate = normalize_to_rate
247362
self._cached_state: Any | None = None
363+
self._overlay_unit_key: OverlayUnitKey | None = None
248364
self._presenters: weakref.WeakSet[PresenterBase] = weakref.WeakSet()
249365
self.autoscaler_kwargs = kwargs
250366
self.autoscalers: dict[ResultKey, Autoscaler] = {}
@@ -257,6 +373,25 @@ def __init__(
257373
# with responsive mode in Panel containers (upstream bug).
258374
self._sizing_opts: dict[str, Any] = {'responsive': True}
259375

376+
@property
377+
def overlay_unit_key(self) -> OverlayUnitKey | None:
378+
"""Unit signature from the last compute(), for cross-layer validation."""
379+
return self._overlay_unit_key
380+
381+
def _extract_overlay_unit_key(
382+
self, data: dict[ResultKey, sc.DataArray]
383+
) -> OverlayUnitKey | None:
384+
"""Extract an overlay unit key from input data.
385+
386+
Default implementation uses dimension coordinates and value unit from the
387+
first DataArray. Subclasses with non-standard coordinate structures (e.g.,
388+
ROI plotters) should override this method.
389+
"""
390+
if not data:
391+
return None
392+
da = next(iter(data.values()))
393+
return OverlayUnitKey.from_data_array(da)
394+
260395
@staticmethod
261396
def _make_tick_opts(tick_params: TickParams | None) -> dict[str, Any]:
262397
"""
@@ -430,9 +565,13 @@ def compute(
430565
if self._normalize_to_rate:
431566
data = {key: _normalize_to_rate(da) for key, da in data.items()}
432567

568+
self._overlay_unit_key = self._extract_overlay_unit_key(data)
569+
433570
resolver = title_resolver or TitleResolver()
434571
plots: list[hv.Element] = []
435572
try:
573+
if self.layout_params.combine_mode == 'overlay':
574+
_validate_overlay_units(data)
436575
for data_key, da in data.items():
437576
label = resolver.get_legend_label(
438577
data_key.job_id.source_name, data_key.output_name

src/ess/livedata/dashboard/roi_readback_plots.py

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,23 @@
2121
)
2222
from ess.livedata.config.workflow_spec import ResultKey
2323

24-
from .plots import Plotter
24+
from .plots import OverlayUnitKey, Plotter, _unit_str
25+
26+
27+
def _roi_overlay_unit_key(da: sc.DataArray) -> OverlayUnitKey:
28+
"""Build an overlay unit key from ROI coordinate metadata.
29+
30+
ROI DataArrays use named coords ('x', 'y') rather than dimension
31+
coordinates, and have no meaningful value unit.
32+
"""
33+
kdim_units = tuple(
34+
sorted(
35+
(name, _unit_str(da.coords[name].unit))
36+
for name in ('x', 'y')
37+
if name in da.coords
38+
)
39+
)
40+
return OverlayUnitKey(kdim_units=kdim_units, vdim_unit=None)
2541

2642

2743
class ROIReadbackStyle(pydantic.BaseModel):
@@ -81,6 +97,14 @@ def from_params(cls, params: RectanglesReadbackParams) -> RectanglesReadbackPlot
8197
"""Create plotter from params."""
8298
return cls(params)
8399

100+
def _extract_overlay_unit_key(
101+
self, data: dict[ResultKey, sc.DataArray]
102+
) -> OverlayUnitKey | None:
103+
if not data:
104+
return None
105+
da = next(iter(data.values()))
106+
return _roi_overlay_unit_key(da)
107+
84108
def plot(
85109
self, data: sc.DataArray, data_key: ResultKey, *, label: str = '', **kwargs
86110
) -> hv.Rectangles:
@@ -187,6 +211,14 @@ def from_params(cls, params: PolygonsReadbackParams) -> PolygonsReadbackPlotter:
187211
"""Create plotter from params."""
188212
return cls(params)
189213

214+
def _extract_overlay_unit_key(
215+
self, data: dict[ResultKey, sc.DataArray]
216+
) -> OverlayUnitKey | None:
217+
if not data:
218+
return None
219+
da = next(iter(data.values()))
220+
return _roi_overlay_unit_key(da)
221+
190222
def plot(
191223
self, data: sc.DataArray, data_key: ResultKey, *, label: str = '', **kwargs
192224
) -> hv.Polygons:

src/ess/livedata/dashboard/roi_request_plots.py

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@
4848
get_roi_mapper,
4949
)
5050

51-
from .plots import Plotter, PresenterBase
51+
from .plots import OverlayUnitKey, Plotter, PresenterBase
5252
from .static_plots import Color, LineDash, RectanglesCoordinates
5353

5454
if TYPE_CHECKING:
@@ -626,6 +626,16 @@ def compute(
626626
else None
627627
)
628628

629+
# Build overlay unit key from ROI coordinate units
630+
kdim_units = tuple(
631+
sorted(
632+
(name, unit)
633+
for name, unit in [('x', self._x_unit), ('y', self._y_unit)]
634+
if unit is not None
635+
)
636+
)
637+
self._overlay_unit_key = OverlayUnitKey(kdim_units=kdim_units, vdim_unit=None)
638+
629639
# Forward data (presenter may use in future)
630640
self._set_cached_state(data)
631641
return data

src/ess/livedata/dashboard/widgets/plot_grid_tabs.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
SubscriptionId,
3636
)
3737
from ..plot_params import PlotAspectType, StretchMode
38+
from ..plots import OverlayUnitKey, validate_cross_layer_units
3839
from ..save_filename import build_save_filename_from_cell, make_save_filename_hook
3940
from ..session_layer import SessionLayer
4041
from ..session_updater import SessionUpdater
@@ -940,6 +941,7 @@ def _get_session_composed_plot(
940941
Composed plot from session DMaps/elements, or None if none available.
941942
"""
942943
plots = []
944+
unit_keys: list[tuple[str, OverlayUnitKey]] = []
943945
has_layout = False
944946
for layer in cell.layers:
945947
layer_id = layer.layer_id
@@ -967,9 +969,22 @@ def _get_session_composed_plot(
967969
has_layout = True
968970
plots.append(dmap)
969971

972+
# Collect unit key for cross-layer validation
973+
if state is not None and state.plotter is not None:
974+
key = state.plotter.overlay_unit_key
975+
if key is not None:
976+
unit_keys.append((layer.config.plot_name, key))
977+
970978
if not plots:
971979
return None
972980

981+
# Validate cross-layer unit compatibility before composing
982+
error = validate_cross_layer_units(unit_keys)
983+
if error is not None:
984+
return hv.Text(0.5, 0.5, f"Error: {error}").opts(
985+
text_align='center', text_baseline='middle'
986+
)
987+
973988
result: hv.DynamicMap | hv.Element
974989
if len(plots) == 1:
975990
result = plots[0]

0 commit comments

Comments
 (0)