From 4acf964cb1166240664c42ecbc9d3c8a56c318c1 Mon Sep 17 00:00:00 2001 From: Simon Heybrock Date: Mon, 9 Nov 2026 08:55:43 +0000 Subject: [PATCH 1/2] loki: load the detector once, and read it by reference Masks and the beam centre each returned a full detector image, so that a plot could draw them. On a hosted service that image would be read for every trial of a mask, and every record held its own. CALIBRATED_DETECTOR now loads the detector once. MASKS and BEAM_CENTRE take it as Referenced[DetectorImage] and return only their small outputs. The record's request names the detector its outputs were computed from, so nothing else has to keep the two in step. These specs take no file of the run, which could differ from the detector, so the detector carries the detector numbers and the sample and source positions they need. CREATE_MASK is a new example of such a spec: it masks ranges of the angle to the beam, and adds to other masks. A configured plot is now a Plot, whose inputs name the referenced values it draws on. A plot whose input is no longer readable shows why as text; the person reruns the detector. Co-Authored-By: Claude Opus 5.5 --- docs/developer/README.md | 3 +- .../essapps/src/ess/apps/panel/outputs.py | 30 ++-- packages/essapps/src/ess/apps/plotting.py | 27 +++- packages/essapps/tests/panel/outputs_test.py | 67 +++++++- packages/essinstruments/docs/loki-app.ipynb | 8 +- .../essinstruments/docs/loki-reduction.ipynb | 102 ++++++++---- .../src/ess/instruments/loki/__init__.py | 2 +- .../src/ess/instruments/loki/bindings.py | 92 +++++++++-- .../src/ess/instruments/loki/plots.py | 40 +++-- .../src/ess/instruments/loki/specs.py | 88 +++++++++-- .../essinstruments/tests/loki/app_test.py | 50 ++++-- .../tests/loki/bindings_test.py | 147 ++++++++++++++---- .../essinstruments/tests/loki/client_test.py | 85 +++++++--- .../essinstruments/tests/loki/specs_test.py | 10 +- 14 files changed, 597 insertions(+), 154 deletions(-) diff --git a/docs/developer/README.md b/docs/developer/README.md index cc80cddd..a2b8c03a 100644 --- a/docs/developer/README.md +++ b/docs/developer/README.md @@ -142,7 +142,8 @@ client.failure(result) # why it failed, or None An application learns what an output is from `client.describe`, which gives the sizes, unit, and coordinate names of each array field and reads no array, and then reads the part it shows with `select=`. A value that was persisted and is no longer in memory is read from the store to be described. The plot of an array of more than two dimensions has a slider for each dimension but the last two, and moving it reads one cut (`ess.apps.panel.Outputs`; which dimensions get a slider is decided in `ess.apps.plotting`, which needs no widget library). -A workflow package can configure plots for its spec: `App(client, plots={spec_id: {'Centre on image': function}})`, where a function takes an instance of the spec's outputs model and returns a plopp figure. +A workflow package can configure plots for its spec: `App(client, plots={spec_id: {'Centre on image': Plot(function, inputs=('detector',))}})`, where the function takes an instance of the spec's outputs model, and returns a plopp figure. +Each of `inputs` names a parameter that references an output, and is passed by name as that value, so that a small output, such as a mask, is drawn on the large value it was computed from, without holding a copy. A request is checked in two phases: diff --git a/packages/essapps/src/ess/apps/panel/outputs.py b/packages/essapps/src/ess/apps/panel/outputs.py index 0d18943e..f400dddf 100644 --- a/packages/essapps/src/ess/apps/panel/outputs.py +++ b/packages/essapps/src/ess/apps/panel/outputs.py @@ -24,8 +24,9 @@ and drawn whole. A field of more dimensions has a slider for each dimension but the last two, and a cut of it is read with ``select=`` for the positions of the sliders, so no more of the array is read than is drawn. A plot configured for a -spec is a function of the record's outputs, every one of them read: it returns a -plopp figure, or fails, which the view shows as text. +spec (:class:`~ess.apps.plotting.Plot`) is a function of the record's outputs, every +one of them read, and of the values its request referenced that it declares: it +returns a plopp figure, or fails, which the view shows as text. Removing an output releases its record (``client.release``), so the record stays in the proposal's history and in the graphs of the records that read it. @@ -40,12 +41,11 @@ import panel as pn import plopp as pp from plopp.core.typing import FigureLike as Figure -from pydantic import BaseModel from pydantic.fields import FieldInfo from ess.apps import ArrayInfo, Client, Listing, NotReadable, Record, SpecId, Status -from ess.apps.plotting import slider_dims -from ess.spec import field_types +from ess.apps.plotting import Plot, slider_dims +from ess.spec import OutputRef, field_types from .graph import Graph from .table import Table, with_sidebar @@ -53,9 +53,8 @@ _PLOT = 'Plot' _PROVENANCE = 'Provenance' -Plots = Mapping[SpecId, Mapping[str, Callable[[BaseModel], Figure]]] -"""Plots by the name of a view, for the records of a spec: each a function of the -record's outputs, as an instance of ``spec.outputs``.""" +Plots = Mapping[SpecId, Mapping[str, Plot]] +"""Plots by the name of a view, for the records of a spec.""" class Outputs: @@ -69,7 +68,7 @@ class Outputs: while its record is pending or completed, and when its record has failed, until removed. ``refresh`` is called once an output is removed; by default it lists the outputs again. ``plots`` holds, for the spec of a - record, functions by the name of the view that shows their figure. + record, the plots by the name of the view that shows their figure. """ def __init__( @@ -218,16 +217,25 @@ def _configured(self, record: Record, view: str) -> Any: if self._listing.status[record.id] is not Status.COMPLETED: return pn.pane.Markdown('') try: - make = self._plots[record.spec][view] + plot = self._plots[record.spec][view] model = self._listing.specs[record.id].outputs values = self._client.output(record) outputs = model.model_construct( **{n: values.get(n) for n in model.model_fields} ) - return _figure_pane(make(outputs)) + inputs = {name: self._input(record, name) for name in plot.inputs} + return _figure_pane(plot.function(outputs, **inputs)) except Exception as error: # a plot is code of the workflow's author return pn.pane.Markdown(f'{view} failed: `{type(error).__name__}: {error}`') + def _input(self, record: Record, name: str) -> Any: + """The value that the param ``name`` of the record's request references.""" + ref = record.request.params[name] + if not isinstance(ref, OutputRef) or ref.key is not None: + raise ValueError(f'{name} does not reference a whole output') + (source,) = self._client.records(ids=[ref.record]) + return self._client.output(source, ref.output) + def _plot(self, record: Record, name: str) -> list[Any]: """ A figure for each array field of the output; a field of more than two diff --git a/packages/essapps/src/ess/apps/plotting.py b/packages/essapps/src/ess/apps/plotting.py index 70e5b74c..5c3f38ea 100644 --- a/packages/essapps/src/ess/apps/plotting.py +++ b/packages/essapps/src/ess/apps/plotting.py @@ -2,7 +2,8 @@ # Copyright (c) 2026 Scipp contributors (https://github.com/scipp) """ What an application decides about plots before it draws any, which needs no -widget library: which dimensions of an array get a slider. +widget library: which dimensions of an array get a slider, and which values a plot +configured for a spec reads. An application learns what an output is from :meth:`~ess.apps.Client.describe`, which reads no array, and reads the part of an array that it draws with @@ -11,7 +12,29 @@ from __future__ import annotations -from collections.abc import Mapping +from collections.abc import Callable, Mapping +from dataclasses import dataclass +from typing import Any + + +@dataclass(frozen=True) +class Plot: + """ + A plot configured for the records of a spec. + + ``function`` takes the record's outputs, as an instance of ``spec.outputs``, and + returns a figure. ``inputs`` names params that hold a reference to a whole output, + each passed to ``function`` by that name as the value it references. So a workflow + with a small output, such as a mask, draws it on the large value that it was + computed from, such as a detector, which its record names:: + + Plot(masks_on_image, inputs=('detector',)) + + An input that is no longer readable fails the plot. + """ + + function: Callable[..., Any] + inputs: tuple[str, ...] = () def slider_dims(sizes: Mapping[str, int]) -> list[str]: diff --git a/packages/essapps/tests/panel/outputs_test.py b/packages/essapps/tests/panel/outputs_test.py index 77bb242e..4d01b14a 100644 --- a/packages/essapps/tests/panel/outputs_test.py +++ b/packages/essapps/tests/panel/outputs_test.py @@ -24,6 +24,7 @@ from ess.apps import Client, Listing, Record, SpecId, Status, local from ess.apps.panel import App, Outputs, Records from ess.apps.panel.records import describe +from ess.apps.plotting import Plot from ess.apps.testing import FakeFiles, RecordingClient, read_file from ess.spec import ( FileRef, @@ -607,7 +608,7 @@ def test_plots_configured_for_a_spec_are_views_of_the_outputs_of_its_records( scale = client.submit(SCALE, params={'curve': curve.ref('curve')}) client.wait([curve, scale]) seen = Seen() - plots = {SpecId.of(CURVE): {'Counts': seen.counts}} + plots = {SpecId.of(CURVE): {'Counts': Plot(seen.counts)}} outputs = Outputs(client, Listing(client), plots=plots) assert outputs.views.options == ['Plot', 'Provenance'] @@ -634,7 +635,7 @@ def test_a_view_the_record_does_not_offer_falls_back_to_the_plot( curve = client.submit(CURVE, params={'run': file}) scale = client.submit(SCALE, params={'curve': curve.ref('curve')}) client.wait([curve, scale]) - plots = {SpecId.of(CURVE): {'Counts': Seen().counts}} + plots = {SpecId.of(CURVE): {'Counts': Plot(Seen().counts)}} outputs = Outputs(client, Listing(client), plots=plots) outputs.select(curve, 'curve') outputs.views.value = 'Counts' @@ -654,7 +655,7 @@ def test_a_configured_plot_that_fails_shows_why_as_text( curve = client.submit(CURVE, params={'run': file}) client.wait(curve) outputs = Outputs( - client, Listing(client), plots={SpecId.of(CURVE): {'Bad': explode}} + client, Listing(client), plots={SpecId.of(CURVE): {'Bad': Plot(explode)}} ) outputs.select(curve, 'curve') @@ -665,8 +666,64 @@ def test_a_configured_plot_that_fails_shows_why_as_text( assert 'RuntimeError: no plot today' in text.object +def scale_on_curve(out: ScaleOutputs, curve: Curve) -> Any: + return pp.plot(curve.counts, title=f'{out.total.__class__.__name__} on curve') + + +@pytest.fixture +def scale_view(client: Client, file: FileRef) -> tuple[Outputs, Record, Record]: + """The outputs with a view of a scale that draws the curve it referenced.""" + curve = client.submit(CURVE, params={'run': file}) + scale = client.submit(SCALE, params={'curve': curve.ref('curve')}) + client.wait([curve, scale]) + plots = {SpecId.of(SCALE): {'On curve': Plot(scale_on_curve, inputs=('curve',))}} + return Outputs(client, Listing(client), plots=plots), curve, scale + + +def test_a_configured_plot_takes_the_value_its_record_referenced( + scale_view: tuple[Outputs, Record, Record], +) -> None: + outputs, _, scale = scale_view + outputs.select(scale, 'total') + + outputs.views.value = 'On curve' + + (pane,) = outputs.view.objects + assert isinstance(pane, pn.pane.Matplotlib), pane.object + assert pane.object.axes[0].get_title() == 'Total on curve' + + +def test_a_configured_plot_whose_input_is_released_shows_why_as_text( + client: Client, scale_view: tuple[Outputs, Record, Record] +) -> None: + outputs, curve, scale = scale_view + client.release(curve) + outputs.select(scale, 'total') + + outputs.views.value = 'On curve' + + (text,) = outputs.view.objects + assert 'On curve failed' in text.object + assert 'NotReadable' in text.object + + +def test_a_configured_plot_input_must_reference_an_output( + client: Client, file: FileRef +) -> None: + curve = client.submit(CURVE, params={'run': file}) + client.wait(curve) + plots = {SpecId.of(CURVE): {'Counts': Plot(Seen().counts, inputs=('run',))}} + outputs = Outputs(client, Listing(client), plots=plots) + outputs.select(curve, 'curve') + + outputs.views.value = 'Counts' + + (text,) = outputs.view.objects + assert 'run does not reference a whole output' in text.object + + def test_a_configured_plot_cannot_take_the_name_of_a_view_of_every_record() -> None: - plots = {SpecId.of(CURVE): {'Plot': explode}} + plots = {SpecId.of(CURVE): {'Plot': Plot(explode)}} with pytest.raises(ValueError, match="curve/v1: \\['Plot'\\]"): Outputs(None, None, plots=plots) # type: ignore[arg-type] @@ -675,7 +732,7 @@ def test_the_app_passes_its_plots_to_the_outputs(client: Client, file: FileRef) curve = client.submit(CURVE, params={'run': file}) client.wait(curve) seen = Seen() - app = App(client, plots={SpecId.of(CURVE): {'Counts': seen.counts}}) + app = App(client, plots={SpecId.of(CURVE): {'Counts': Plot(seen.counts)}}) app.poll() app.outputs.select(curve, 'curve') diff --git a/packages/essinstruments/docs/loki-app.ipynb b/packages/essinstruments/docs/loki-app.ipynb index a42871d1..3b911189 100644 --- a/packages/essinstruments/docs/loki-app.ipynb +++ b/packages/essinstruments/docs/loki-app.ipynb @@ -96,9 +96,11 @@ "\n", "The tab *Workflows* holds the forms, the tab *Outputs* the results.\n", "\n", - "1. **Pixel masks**: pick the form *Pixel masks*, give run 60339 and the mask file, and submit. In the tab *Outputs*, select the image and pick the view *Masks on image*.\n", - "2. **Beam centre**: pick the form *Beam centre*, give run 60339 and the masks as *From a record*, and submit. The view *Centre on image* marks the centre.\n", - "3. **I(Q)**: pick the form *I(Q)*, give a sample run and its transmission run, the can, its transmission run, the empty beam, the direct beam, the masks and the beam centre *From a record*, and the bins, and submit. The view *I(Q)* draws the result. The form keeps its values, so for the next sample change only the two sample fields." + "1. **Calibrated detector**: pick the form *Calibrated detector*, give run 60339, and submit.\n", + "2. **Pixel masks**: pick the form *Pixel masks*, give the detector as *From a record* and the mask file, and submit. In the tab *Outputs*, select the masks and pick the view *Masks on image*.\n", + "3. **Beam centre**: pick the form *Beam centre*, give the detector and the masks as *From a record*, and submit. The view *Centre on image* marks the centre.\n", + "4. **Create mask**, if needed: pick the form *Create mask*, give the detector, the beam centre, and the masks as *From a record*, add angle ranges, and submit. The view *Masks on image* leaves their pixels out. Use the result as the masks of I(Q).\n", + "5. **I(Q)**: pick the form *I(Q)*, give a sample run and its transmission run, the can, its transmission run, the empty beam, the direct beam, the masks and the beam centre *From a record*, and the bins, and submit. The view *I(Q)* draws the result. The form keeps its values, so for the next sample change only the two sample fields." ] }, { diff --git a/packages/essinstruments/docs/loki-reduction.ipynb b/packages/essinstruments/docs/loki-reduction.ipynb index 7e3619cd..c5153f7b 100644 --- a/packages/essinstruments/docs/loki-reduction.ipynb +++ b/packages/essinstruments/docs/loki-reduction.ipynb @@ -8,12 +8,14 @@ "# LoKI reduction on `local()`\n", "\n", "The specs and bindings of `ess.instruments.loki` on the runs of the LoKI detector test at Larmor, without a GUI.\n", - "A reduction is three records, and a person can look at each before the next:\n", + "A reduction is a chain of records, and a person can look at each before the next:\n", "\n", "```\n", - "mask files, one run ──> masks ──> beam centre ──> I(Q)\n", - " └───────────────────────^\n", - "```" + "one run ──> detector ──> masks ──> beam centre ──> I(Q)\n", + " mask files ─────────────^ └────────────────────^\n", + "```\n", + "\n", + "The detector is loaded once, and the masks and the beam centre read it by reference." ] }, { @@ -43,7 +45,13 @@ "\n", "from ess.apps import FolderFiles, Template, file, local\n", "from ess.instruments.loki.bindings import BINDINGS\n", - "from ess.instruments.loki.specs import BEAM_CENTRE, IOFQ, MASKS\n", + "from ess.instruments.loki.specs import (\n", + " BEAM_CENTRE,\n", + " CALIBRATED_DETECTOR,\n", + " CREATE_MASK,\n", + " IOFQ,\n", + " MASKS,\n", + ")\n", "\n", "for download in (\n", " data.loki_tutorial_run_60392,\n", @@ -102,10 +110,11 @@ "id": "4", "metadata": {}, "source": [ - "## Masks\n", + "## Detector and masks\n", "\n", - "`MASKS` reads the mask files and shows them on the detector image of a run.\n", - "The image holds the counts per pixel, with each pixel's position and the masks, so a plot can leave the masked pixels out or show what they hold." + "`CALIBRATED_DETECTOR` loads the detector of a run: the counts per pixel, with each pixel's position.\n", + "`MASKS` reads the mask files for the pixels of that detector, and returns only the masks.\n", + "Its request names the detector, so a plot can draw the masks on the detector they were computed from." ] }, { @@ -115,11 +124,15 @@ "metadata": {}, "outputs": [], "source": [ - "masks = client.submit(\n", - " MASKS, params={'run': file(run=60339), 'mask_files': [file(path=mask_file)]}\n", + "detector = client.compute(CALIBRATED_DETECTOR, params={'run': file(run=60339)})\n", + "masks = client.compute(\n", + " MASKS,\n", + " params={\n", + " 'detector': detector.ref('detector'),\n", + " 'mask_files': [file(path=mask_file)],\n", + " },\n", ")\n", - "client.wait(masks)\n", - "image = client.output(masks, 'image').counts\n", + "image = client.output(detector, 'detector').counts\n", "image" ] }, @@ -132,7 +145,7 @@ "source": [ "from ess.instruments.loki.plots import on_detector\n", "\n", - "sc.plot(on_detector(image, masked=False), norm='log', title='All pixels')" + "sc.plot(on_detector(image), norm='log', title='All pixels')" ] }, { @@ -142,7 +155,8 @@ "metadata": {}, "outputs": [], "source": [ - "sc.plot(on_detector(image), norm='log', title='Without the masked pixels')" + "masked = image.assign_masks(client.output(masks, 'masks').masks)\n", + "sc.plot(on_detector(masked), norm='log', title='Without the masked pixels')" ] }, { @@ -153,7 +167,7 @@ "## Beam centre\n", "\n", "`BEAM_CENTRE` finds the centre of mass of the counts outside the masks: a point on the beam relative to the sample, in the same frame as the image's positions, with `z` the distance from the sample.\n", - "The plot marks the centre on the image, from two outputs of one record." + "The plot marks the centre on the image of the detector it was found from." ] }, { @@ -164,7 +178,8 @@ "outputs": [], "source": [ "centre = client.submit(\n", - " BEAM_CENTRE, params={'sample': file(run=60339), 'masks': masks.ref('masks')}\n", + " BEAM_CENTRE,\n", + " params={'detector': detector.ref('detector'), 'masks': masks.ref('masks')},\n", ")\n", "client.wait(centre)\n", "found = client.output(centre, 'centre')\n", @@ -179,7 +194,7 @@ "outputs": [], "source": [ "figure = sc.plot(\n", - " on_detector(client.output(centre, 'image').counts),\n", + " on_detector(masked),\n", " norm='log',\n", " title=f'Beam centre: x = {found.x:.4f} m, y = {found.y:.4f} m',\n", ")\n", @@ -191,6 +206,41 @@ "cell_type": "markdown", "id": "11", "metadata": {}, + "source": [ + "## An angle range\n", + "\n", + "`CREATE_MASK` masks the pixels within the ranges of the angle to the beam, as seen from the sample.\n", + "The angle needs the beam centre, so it follows `BEAM_CENTRE`, and it adds to the masks that the centre was found with.\n", + "`IOFQ` takes the result in place of `masks`." + ] + }, + { + "cell_type": "code", + "execution_count": null, + "id": "12", + "metadata": {}, + "outputs": [], + "source": [ + "angles = client.compute(\n", + " CREATE_MASK,\n", + " params={\n", + " 'detector': detector.ref('detector'),\n", + " 'beam_centre': centre.ref('centre'),\n", + " 'base': masks.ref('masks'),\n", + " 'shapes': [{'start': 0.0, 'stop': 1.0}],\n", + " },\n", + ")\n", + "sc.plot(\n", + " on_detector(image.assign_masks(client.output(angles, 'masks').masks)),\n", + " norm='log',\n", + " title='Also without the pixels within 1 degree of the beam',\n", + ")" + ] + }, + { + "cell_type": "markdown", + "id": "13", + "metadata": {}, "source": [ "## I(Q)\n", "\n", @@ -200,7 +250,7 @@ { "cell_type": "code", "execution_count": null, - "id": "12", + "id": "14", "metadata": {}, "outputs": [], "source": [ @@ -229,7 +279,7 @@ { "cell_type": "code", "execution_count": null, - "id": "13", + "id": "15", "metadata": {}, "outputs": [], "source": [ @@ -239,7 +289,7 @@ }, { "cell_type": "markdown", - "id": "14", + "id": "16", "metadata": {}, "source": [ "## A batch with one can\n", @@ -251,7 +301,7 @@ { "cell_type": "code", "execution_count": null, - "id": "15", + "id": "17", "metadata": {}, "outputs": [], "source": [ @@ -283,7 +333,7 @@ }, { "cell_type": "markdown", - "id": "16", + "id": "18", "metadata": {}, "source": [ "## Tweaking one run\n", @@ -294,7 +344,7 @@ { "cell_type": "code", "execution_count": null, - "id": "17", + "id": "19", "metadata": {}, "outputs": [], "source": [ @@ -323,7 +373,7 @@ }, { "cell_type": "markdown", - "id": "18", + "id": "20", "metadata": {}, "source": [ "## What a result came from\n", @@ -334,7 +384,7 @@ { "cell_type": "code", "execution_count": null, - "id": "19", + "id": "21", "metadata": {}, "outputs": [], "source": [ @@ -345,7 +395,7 @@ { "cell_type": "code", "execution_count": null, - "id": "20", + "id": "22", "metadata": {}, "outputs": [], "source": [ diff --git a/packages/essinstruments/src/ess/instruments/loki/__init__.py b/packages/essinstruments/src/ess/instruments/loki/__init__.py index bf1aaa86..e1fb4e3f 100644 --- a/packages/essinstruments/src/ess/instruments/loki/__init__.py +++ b/packages/essinstruments/src/ess/instruments/loki/__init__.py @@ -1,7 +1,7 @@ # SPDX-License-Identifier: BSD-3-Clause # Copyright (c) 2026 Scipp contributors (https://github.com/scipp) """ -LoKI's reductions: pixel masks, beam centre, and I(Q). +LoKI's reductions: a detector, pixel masks, beam centre, and I(Q). :mod:`~ess.instruments.loki.specs` needs no workflow package, and :mod:`~ess.instruments.loki.bindings` needs the extra ``[loki]``. diff --git a/packages/essinstruments/src/ess/instruments/loki/bindings.py b/packages/essinstruments/src/ess/instruments/loki/bindings.py index dd991d85..8353c799 100644 --- a/packages/essinstruments/src/ess/instruments/loki/bindings.py +++ b/packages/essinstruments/src/ess/instruments/loki/bindings.py @@ -3,19 +3,23 @@ """ The bindings of LoKI's specs over esssans, for the LoKI detector test at Larmor. -The masks and the beam centre are functions, since esssans builds its masks by -changing a pipeline, and its beam-centre finder takes a pipeline. I(Q) is a +The detector, the masks, and the beam centre are functions, since esssans builds its +masks by changing a pipeline, and its beam-centre finder takes a pipeline. Those that +read the detector give esssans what the detector holds in place of the file. I(Q) is a pipeline, so that it serves stages. Its masks are a value given to the pipeline, which replaces the provider of esssans that reads them from a file and the sample run. """ from collections.abc import Mapping +from functools import reduce +from operator import or_ from pathlib import Path from typing import Any import sciline import scipp as sc +import scippnexus as snx from ess import sans from ess.loki.larmor_workflow import LokiAtLarmorWorkflow from ess.sans.types import ( @@ -24,6 +28,7 @@ BeamCenter, CorrectedDetector, CorrectForGravity, + DetectorIDs, DetectorMasks, DirectBeam, DirectBeamFilename, @@ -31,6 +36,7 @@ Filename, Measurement, Numerator, + Position, QBins, ReturnEvents, SampleRun, @@ -47,12 +53,17 @@ from .specs import ( BEAM_CENTRE, + CALIBRATED_DETECTOR, + CREATE_MASK, IOFQ, MASKS, BeamCentre, BeamCentreOutputs, BeamCentreParams, + CalibratedDetectorOutputs, + CalibratedDetectorParams, Corrections, + CreateMaskParams, DetectorImage, IofQ, MasksOutputs, @@ -64,6 +75,9 @@ _NO_CORRECTION = sc.vector([0.0, 0.0, 0.0], unit='m') """The value of esssans' key that applies no beam-centre correction.""" +_ANGLE_RANGES = 'angle ranges' +"""The name of the mask of the angle ranges of a record.""" + def _run_workflow(run: Path) -> sciline.Pipeline: """The workflow of the detector of one run, with no beam-centre correction.""" @@ -73,37 +87,77 @@ def _run_workflow(run: Path) -> sciline.Pipeline: return workflow -def _image(detector: sc.DataArray) -> DetectorImage: - return DetectorImage(counts=detector.bins.sum()) +def load_detector(params: CalibratedDetectorParams) -> CalibratedDetectorOutputs: + workflow = _run_workflow(params.run) + workflow[DetectorMasks] = {} + detector = CorrectedDetector[SampleRun, Numerator] + sample = Position[snx.NXsample, SampleRun] + source = Position[snx.NXsource, SampleRun] + values = workflow.compute((detector, sample, source)) + counts = values[detector].bins.sum() + return CalibratedDetectorOutputs( + detector=DetectorImage( + counts=counts.assign_coords( + sample_position=values[sample], source_position=values[source] + ) + ) + ) def compute_masks(params: MasksParams) -> MasksOutputs: - workflow = _run_workflow(params.run) masks: dict[str, sc.Variable] = {} if params.mask_files: + workflow = LokiAtLarmorWorkflow() + workflow[DetectorIDs] = params.detector.counts.coords['detector_number'] # esssans names each mask by the path of its file, which differs by host. by_path = sans.with_pixel_mask_filenames( workflow, [str(path) for path in params.mask_files] ).compute(DetectorMasks) masks = {Path(path).name: mask for path, mask in by_path.items()} - workflow[DetectorMasks] = masks - detector = workflow.compute(CorrectedDetector[SampleRun, Numerator]) - return MasksOutputs(masks=PixelMasks(masks=masks), image=_image(detector)) + return MasksOutputs(masks=PixelMasks(masks=masks)) def find_beam_centre(params: BeamCentreParams) -> BeamCentreOutputs: - workflow = _run_workflow(params.sample) - workflow[DetectorMasks] = params.masks.masks - key = CorrectedDetector[SampleRun, Numerator] - detector = workflow.compute(key) - workflow[key] = detector # so that the finder does not load the run again + counts = params.detector.counts + workflow = LokiAtLarmorWorkflow() + workflow[Position[snx.NXsample, SampleRun]] = counts.coords['sample_position'] + workflow[Position[snx.NXsource, SampleRun]] = counts.coords['source_position'] + # The finder sums the dimensions that the positions lack, and else expects events. + workflow[CorrectedDetector[SampleRun, Numerator]] = sc.concat( + [counts.assign_masks(params.masks.masks)], 'summed' + ) centre = sans.beam_center_from_center_of_mass(workflow).to(unit='m') return BeamCentreOutputs( centre=BeamCentre( x=centre.fields.x.value, y=centre.fields.y.value, z=centre.fields.z.value + ) + ) + + +def _angle_to_beam(counts: sc.DataArray, centre: BeamCentre) -> sc.Variable: + """The angle of each pixel to the beam, seen from the sample, in degrees.""" + to_pixel = counts.coords['position'] - counts.coords['sample_position'] + beam = sc.vector([centre.x, centre.y, centre.z], unit='m') + angle = sc.atan2(y=sc.norm(sc.cross(beam, to_pixel)), x=sc.dot(beam, to_pixel)) + return angle.to(unit='deg') + + +def create_mask(params: CreateMaskParams) -> MasksOutputs: + angle = _angle_to_beam(params.detector.counts, params.beam_centre) + mask = reduce( + or_, + ( + (angle >= sc.scalar(s.start, unit='deg')) + & (angle < sc.scalar(s.stop, unit='deg')) + for s in params.shapes ), - image=_image(detector), ) + base = {} if params.base is None else params.base.masks + masks = {name: value.copy() for name, value in base.items()} + if _ANGLE_RANGES in masks: + mask = mask | masks[_ANGLE_RANGES] + masks[_ANGLE_RANGES] = mask + return MasksOutputs(masks=PixelMasks(masks=masks)) def _detector_masks(masks: PixelMasks) -> DetectorMasks: @@ -166,8 +220,10 @@ def _iofq_pipeline(fixed: Mapping[str, Any]) -> sciline.Pipeline: return pipeline +detector_binding = function_binding(CALIBRATED_DETECTOR, load_detector) masks_binding = function_binding(MASKS, compute_masks) beam_centre_binding = function_binding(BEAM_CENTRE, find_beam_centre) +create_mask_binding = function_binding(CREATE_MASK, create_mask) iofq_binding = PipelineBinding( IOFQ, _iofq_pipeline, @@ -181,4 +237,10 @@ def _iofq_pipeline(fixed: Mapping[str, Any]) -> sciline.Pipeline: }, ) -BINDINGS = [masks_binding, beam_centre_binding, iofq_binding] +BINDINGS = [ + detector_binding, + masks_binding, + beam_centre_binding, + create_mask_binding, + iofq_binding, +] diff --git a/packages/essinstruments/src/ess/instruments/loki/plots.py b/packages/essinstruments/src/ess/instruments/loki/plots.py index 7bc1ca31..ed8f5112 100644 --- a/packages/essinstruments/src/ess/instruments/loki/plots.py +++ b/packages/essinstruments/src/ess/instruments/loki/plots.py @@ -1,9 +1,10 @@ # SPDX-License-Identifier: BSD-3-Clause # Copyright (c) 2026 Scipp contributors (https://github.com/scipp) """ -The plots of LoKI's specs: each a function of the outputs of one record that -returns a plopp figure, as ``ess.apps.panel.App(plots=)`` takes them. They need -plopp, not Panel, so a notebook can draw them as well. +The plots of LoKI's specs: each a function of the outputs of one record, and of +what its request referenced, that returns a plopp figure, as +``ess.apps.panel.App(plots=)`` takes them. They need plopp, not Panel, so a notebook +can draw them as well. """ import plopp as pp @@ -11,21 +12,24 @@ from plopp.core.typing import FigureLike as Figure from ess.apps import SpecId +from ess.apps.plotting import Plot from .specs import ( BEAM_CENTRE, + CREATE_MASK, IOFQ, MASKS, BeamCentreOutputs, + DetectorImage, IofQOutputs, MasksOutputs, + PixelMasks, ) -def on_detector(image: sc.DataArray, *, masked: bool = True) -> sc.DataArray: +def on_detector(image: sc.DataArray) -> sc.DataArray: """The counts by the position of each pixel on the detector plane.""" - pixels = image if masked else image.drop_masks(list(image.masks)) - pixels = pixels.flatten(to='id') + pixels = image.flatten(to='id') # hist needs contiguous coordinates, and a field is a view with strides. pixels.coords['x'] = pixels.coords['position'].fields.x.copy() pixels.coords['y'] = pixels.coords['position'].fields.y.copy() @@ -33,20 +37,22 @@ def on_detector(image: sc.DataArray, *, masked: bool = True) -> sc.DataArray: return pixels.drop_coords(others).hist(y=128, x=128) -def masks_on_image(outputs: MasksOutputs) -> Figure: +def masks_on_image(outputs: MasksOutputs, detector: DetectorImage) -> Figure: """The detector image without the masked pixels, which are left blank.""" return pp.plot( - on_detector(outputs.image.counts), + on_detector(detector.counts.assign_masks(outputs.masks.masks)), norm='log', title='Without the masked pixels', ) -def centre_on_image(outputs: BeamCentreOutputs) -> Figure: +def centre_on_image( + outputs: BeamCentreOutputs, detector: DetectorImage, masks: PixelMasks +) -> Figure: """The detector image, with a red cross at the beam centre.""" centre = outputs.centre figure = pp.plot( - on_detector(outputs.image.counts), + on_detector(detector.counts.assign_masks(masks.masks)), norm='log', title=f'Beam centre: x = {centre.x:.4f} m, y = {centre.y:.4f} m', ) @@ -65,8 +71,16 @@ def transmission_curves(outputs: IofQOutputs) -> Figure: return pp.plot({'sample': curves.sample, 'can': curves.can}, title='Transmission') +_MASKS_ON_IMAGE = Plot(masks_on_image, inputs=('detector',)) + PLOTS = { - SpecId.of(MASKS): {'Masks on image': masks_on_image}, - SpecId.of(BEAM_CENTRE): {'Centre on image': centre_on_image}, - SpecId.of(IOFQ): {'I(Q)': iofq_curve, 'Transmission': transmission_curves}, + SpecId.of(MASKS): {'Masks on image': _MASKS_ON_IMAGE}, + SpecId.of(CREATE_MASK): {'Masks on image': _MASKS_ON_IMAGE}, + SpecId.of(BEAM_CENTRE): { + 'Centre on image': Plot(centre_on_image, inputs=('detector', 'masks')) + }, + SpecId.of(IOFQ): { + 'I(Q)': Plot(iofq_curve), + 'Transmission': Plot(transmission_curves), + }, } diff --git a/packages/essinstruments/src/ess/instruments/loki/specs.py b/packages/essinstruments/src/ess/instruments/loki/specs.py index a986e1ed..79dc694b 100644 --- a/packages/essinstruments/src/ess/instruments/loki/specs.py +++ b/packages/essinstruments/src/ess/instruments/loki/specs.py @@ -3,15 +3,22 @@ """ The specs of LoKI's reductions. -A reduction is three records, each of which a person may look at before the next:: +A reduction is a chain of records, each of which a person may look at before the +next:: - masks = client.submit(MASKS, params={'run': file(run=60339), + detector = client.submit(CALIBRATED_DETECTOR, params={'run': file(run=60339)}) + masks = client.submit(MASKS, params={'detector': detector.ref('detector'), 'mask_files': [file(path=mask_file)]}) - centre = client.submit(BEAM_CENTRE, params={'sample': file(run=60339), + centre = client.submit(BEAM_CENTRE, params={'detector': detector.ref('detector'), 'masks': masks.ref('masks')}) iofq = client.submit(IOFQ, params={'masks': masks.ref('masks'), 'beam_centre': centre.ref('centre'), ...}) +The detector is loaded once. The specs that read it take it by reference and return +only their small outputs, so that the request of such a record names the detector its +outputs were computed from, and a plot finds the detector there. They take no file of +the run, which could differ from the detector, so the detector holds all they need. + The specs are separate where a value can outlive one sample: the masks hold for an experiment, and one beam centre can serve the samples of a sample changer, though a more invasive change of sample needs its own. I(Q) takes the masks @@ -20,10 +27,10 @@ """ from pathlib import Path -from typing import Annotated, Literal +from typing import Annotated, Literal, Self import scipp as sc -from pydantic import BaseModel, Field +from pydantic import BaseModel, Field, model_validator from ess.spec import ( BackgroundRunFile, @@ -50,7 +57,8 @@ class PixelMasks(BaseModel, arbitrary_types_allowed=True): class DetectorImage(BaseModel, arbitrary_types_allowed=True): """ - The counts per pixel of one run, with each pixel's position and the masks. + The counts per pixel of one run, with each pixel's ``position`` and + ``detector_number``, and the ``sample_position`` and ``source_position`` of the run. The positions are those of the file, not corrected by a beam centre. With the sample at the origin, as in the files of the detector test at Larmor, a beam @@ -106,8 +114,28 @@ class TransmissionCurves(BaseModel, arbitrary_types_allowed=True): can: sc.DataArray +class CalibratedDetectorParams(BaseModel): + run: RunFile = Field(description='The run to load the detector of.') + + +class CalibratedDetectorOutputs(BaseModel): + detector: DetectorImage + + +CALIBRATED_DETECTOR = WorkflowSpec( + name='loki-calibrated-detector', + version=1, + title='Calibrated detector', + description='The counts per pixel of a run, with the position of each pixel.', + params=CalibratedDetectorParams, + outputs=CalibratedDetectorOutputs, +) + + class MasksParams(BaseModel): - run: RunFile = Field(description='The run to show the masks on.') + detector: Referenced[DetectorImage] = Field( + description='The detector whose pixels the masks hold.' + ) mask_files: list[MaskFile] = Field( description='The files of the masks. Without any, no pixel is left out.' ) @@ -115,27 +143,27 @@ class MasksParams(BaseModel): class MasksOutputs(BaseModel): masks: PixelMasks - image: DetectorImage MASKS = WorkflowSpec( name='loki-masks', version=1, title='Pixel masks', - description='Pixel masks from mask files, shown on the detector image of a run.', + description='Pixel masks from mask files.', params=MasksParams, outputs=MasksOutputs, ) class BeamCentreParams(BaseModel): - sample: SampleRunFile = Field(description='The run to find the beam centre from.') + detector: Referenced[DetectorImage] = Field( + description='The detector to find the beam centre from.' + ) masks: Referenced[PixelMasks] = Field(description='The pixels to leave out.') class BeamCentreOutputs(BaseModel): centre: BeamCentre - image: DetectorImage BEAM_CENTRE = WorkflowSpec( @@ -148,6 +176,44 @@ class BeamCentreOutputs(BaseModel): ) +class AngleRange(BaseModel): + """The pixels whose angle to the beam, seen from the sample, is in [start, stop).""" + + start: float = Field(ge=0, description='The lower limit, in degrees.') + stop: float = Field(le=180, description='The upper limit, in degrees.') + + @model_validator(mode='after') + def _stop_follows_start(self) -> Self: + if self.stop <= self.start: + raise ValueError(f'stop {self.stop} must exceed start {self.start}') + return self + + +class CreateMaskParams(BaseModel): + detector: Referenced[DetectorImage] = Field( + description='The detector whose pixels the mask holds.' + ) + beam_centre: BeamCentre | Referenced[BeamCentre] = Field( + description='The beam centre, typed in or found by a beam-centre record.' + ) + base: Referenced[PixelMasks] | None = Field( + description='The masks to add to. None starts without any.' + ) + shapes: list[AngleRange] = Field( + min_length=1, description='The pixels to leave out, any of these.' + ) + + +CREATE_MASK = WorkflowSpec( + name='loki-create-mask', + version=1, + title='Create mask', + description='A pixel mask of the angle ranges given, added to other masks.', + params=CreateMaskParams, + outputs=MasksOutputs, +) + + class IofQParams(BaseModel): sample: SampleRunFile = Field(description='The sample run.') sample_transmission: TransmissionRunFile = Field( diff --git a/packages/essinstruments/tests/loki/app_test.py b/packages/essinstruments/tests/loki/app_test.py index 4cf0201f..1543652f 100644 --- a/packages/essinstruments/tests/loki/app_test.py +++ b/packages/essinstruments/tests/loki/app_test.py @@ -2,7 +2,7 @@ # Copyright (c) 2026 Scipp contributors (https://github.com/scipp) """ The application through its widgets, on the files of esssans' tutorial, in the -order a person uses it: masks, beam centre, and I(Q) of one run. +order a person uses it: detector, masks, beam centre, and I(Q) of one run. """ from collections.abc import Iterator @@ -10,6 +10,7 @@ from pathlib import Path from typing import Any +import numpy as np import panel as pn import pytest from ess.instruments.loki.bindings import BINDINGS @@ -24,8 +25,10 @@ @dataclass class Session: app: App + detector: Record masks: Record centre: Record + angles: Record iofq: Record @@ -51,7 +54,7 @@ def run(number: int, role: str | None = None) -> dict[str, object]: def session( tutorial: FolderFiles, mask_file: Path, direct_beam: Path ) -> Iterator[Session]: - """The app with the masks, the beam centre, and I(Q) of one run computed.""" + """The app with every step, up to I(Q) of one run, computed.""" with local(proposal='p1', files=tutorial, bind=BINDINGS) as client: app = App(client, plots=PLOTS) @@ -64,12 +67,21 @@ def submit(title: str, params: dict[str, Any]) -> Record: app.poll() return record + detector = submit('Calibrated detector', {'run': file(run=60339)}) + image = detector.ref('detector') masks = submit( 'Pixel masks', - {'run': file(run=60339), 'mask_files': [file(path=mask_file)]}, + {'detector': image, 'mask_files': [file(path=mask_file)]}, ) - centre = submit( - 'Beam centre', {'sample': file(run=60339), 'masks': masks.ref('masks')} + centre = submit('Beam centre', {'detector': image, 'masks': masks.ref('masks')}) + angles = submit( + 'Create mask', + { + 'detector': image, + 'beam_centre': centre.ref('centre'), + 'base': masks.ref('masks'), + 'shapes': [{'start': 3.0, 'stop': 180.0}], + }, ) iofq = submit( 'I(Q)', @@ -86,14 +98,16 @@ def submit(title: str, params: dict[str, Any]) -> Record: 'q': {'start': 0.01, 'stop': 0.3, 'num_bins': 100}, }, ) - yield Session(app, masks, centre, iofq) + yield Session(app, detector, masks, centre, angles, iofq) def test_masks_then_centre_then_one_run_are_computed(session: Session) -> None: client = session.app.client + assert client.failure(session.detector) is None assert client.failure(session.masks) is None assert client.failure(session.centre) is None + assert client.failure(session.angles) is None assert client.failure(session.iofq) is None assert session.iofq.request.params['beam_centre'] == session.centre.ref('centre') @@ -102,8 +116,9 @@ def test_the_plots_of_loki_draw_the_outputs_of_real_records(session: Session) -> outputs = session.app.outputs drawn = {} for record, name, view in [ - (session.masks, 'image', 'Masks on image'), - (session.centre, 'image', 'Centre on image'), + (session.masks, 'masks', 'Masks on image'), + (session.angles, 'masks', 'Masks on image'), + (session.centre, 'centre', 'Centre on image'), (session.iofq, 'iofq', 'I(Q)'), (session.iofq, 'transmission', 'Transmission'), ]: @@ -111,9 +126,18 @@ def test_the_plots_of_loki_draw_the_outputs_of_real_records(session: Session) -> outputs.views.value = view (pane,) = outputs.view.objects assert isinstance(pane, pn.pane.Matplotlib), pane.object - drawn[view] = pane.object.axes[0] - - assert any(line.get_marker() == '+' for line in drawn['Centre on image'].lines) - assert drawn['I(Q)'].get_yscale() == 'log' - legend = drawn['Transmission'].get_legend() + drawn[record.id, view] = pane.object.axes[0] + + def of(record: Record, view: str) -> Any: + return drawn[record.id, view] + + file_masks, angle_masks = ( + np.nansum(of(record, 'Masks on image').images[0].get_array()) + for record in (session.masks, session.angles) + ) + assert 0 < angle_masks < file_masks + centre = of(session.centre, 'Centre on image') + assert any(line.get_marker() == '+' for line in centre.lines) + assert of(session.iofq, 'I(Q)').get_yscale() == 'log' + legend = of(session.iofq, 'Transmission').get_legend() assert [t.get_text() for t in legend.get_texts()] == ['sample', 'can'] diff --git a/packages/essinstruments/tests/loki/bindings_test.py b/packages/essinstruments/tests/loki/bindings_test.py index 36964258..cc468b77 100644 --- a/packages/essinstruments/tests/loki/bindings_test.py +++ b/packages/essinstruments/tests/loki/bindings_test.py @@ -11,8 +11,12 @@ import scipp as sc from ess.instruments.loki import bindings from ess.instruments.loki.specs import ( + AngleRange, BeamCentre, BeamCentreParams, + CalibratedDetectorParams, + CreateMaskParams, + DetectorImage, IofQParams, MasksOutputs, MasksParams, @@ -24,43 +28,56 @@ @pytest.fixture(scope='module') -def computed_masks(sample: Path, mask_file: Path) -> MasksOutputs: +def detector(sample: Path) -> DetectorImage: + return bindings.detector_binding.compute( + CalibratedDetectorParams(run=sample) + ).detector + + +@pytest.fixture(scope='module') +def computed_masks(detector: DetectorImage, mask_file: Path) -> MasksOutputs: return bindings.masks_binding.compute( - MasksParams(run=sample, mask_files=[mask_file]) + MasksParams(detector=detector, mask_files=[mask_file]) ) -def test_masks_are_named_by_file_and_shown_on_the_image( - computed_masks: MasksOutputs, mask_file: Path +def test_the_detector_holds_counts_and_what_the_specs_that_read_it_need( + detector: DetectorImage, +) -> None: + counts = detector.counts + assert counts.sum().value > 0 + assert not counts.masks + assert { + 'position', + 'detector_number', + 'sample_position', + 'source_position', + } <= set(counts.coords) + + +def test_masks_are_named_by_file_and_fit_the_detector( + computed_masks: MasksOutputs, detector: DetectorImage, mask_file: Path ) -> None: masks = computed_masks.masks.masks assert list(masks) == [mask_file.name] assert masks[mask_file.name].sum().value == 249022 - image = computed_masks.image.counts - assert sc.identical(image.masks[mask_file.name], masks[mask_file.name]) - assert 'position' in image.coords - assert image.sizes == masks[mask_file.name].sizes + assert detector.counts.sizes == masks[mask_file.name].sizes -def test_without_mask_files_no_pixel_is_masked(sample: Path) -> None: - outputs = bindings.masks_binding.compute(MasksParams(run=sample, mask_files=[])) +def test_without_mask_files_no_pixel_is_masked(detector: DetectorImage) -> None: + outputs = bindings.masks_binding.compute( + MasksParams(detector=detector, mask_files=[]) + ) assert outputs.masks.masks == {} - assert not outputs.image.counts.masks - assert outputs.image.counts.sum().value > 0 - - -def test_a_mask_marks_pixels_on_the_image_and_keeps_their_counts( - computed_masks: MasksOutputs, -) -> None: - image = computed_masks.image.counts - assert image.drop_masks(list(image.masks)).sum().value > image.sum().value def test_beam_centre_is_that_of_the_tutorial( - sample: Path, computed_masks: MasksOutputs, tutorial_centre: BeamCentre + detector: DetectorImage, + computed_masks: MasksOutputs, + tutorial_centre: BeamCentre, ) -> None: outputs = bindings.beam_centre_binding.compute( - BeamCentreParams(sample=sample, masks=computed_masks.masks) + BeamCentreParams(detector=detector, masks=computed_masks.masks) ) assert outputs.centre.x == pytest.approx(tutorial_centre.x) assert outputs.centre.y == pytest.approx(tutorial_centre.y) @@ -68,17 +85,89 @@ def test_beam_centre_is_that_of_the_tutorial( assert outputs.centre.z == pytest.approx(tutorial_centre.z, abs=1e-5) -def test_beam_centre_image_shares_no_memory_with_the_masks_given( - sample: Path, computed_masks: MasksOutputs +def angle_mask( + detector: DetectorImage, + centre: BeamCentre, + ranges: list[AngleRange], + base: PixelMasks | None = None, +) -> PixelMasks: + return bindings.create_mask_binding.compute( + CreateMaskParams( + detector=detector, beam_centre=centre, base=base, shapes=ranges + ) + ).masks + + +def test_an_angle_range_masks_the_pixels_at_those_angles( + detector: DetectorImage, +) -> None: + ranges = [ + AngleRange(start=1.0, stop=2.0), + AngleRange(start=3.0, stop=4.0), + ] + masks = angle_mask(detector, BeamCentre(x=0.0, y=0.0, z=1.0), ranges) + (mask,) = masks.masks.values() + coords = detector.counts.coords + to_pixel = coords['position'] - coords['sample_position'] + angle = np.degrees( + np.arctan2( + np.hypot(to_pixel.fields.x.values, to_pixel.fields.y.values), + to_pixel.fields.z.values, + ) + ) + assert 0 < mask.values.sum() < mask.values.size + assert mask.sizes == detector.counts.sizes + assert np.array_equal( + mask.values, ((1 <= angle) & (angle < 2)) | ((3 <= angle) & (angle < 4)) + ) + + +def test_an_angle_is_measured_from_the_beam_centre( + detector: DetectorImage, tutorial_centre: BeamCentre +) -> None: + masks = angle_mask(detector, tutorial_centre, [AngleRange(start=0.0, stop=1.0)]) + (mask,) = masks.masks.values() + position = detector.counts.coords['position'] + assert position.fields.x.values[mask.values].mean() == pytest.approx( + tutorial_centre.x, abs=2e-3 + ) + assert position.fields.y.values[mask.values].mean() == pytest.approx( + tutorial_centre.y, abs=2e-3 + ) + + +def test_an_angle_mask_adds_to_the_masks_given_and_shares_no_memory_with_them( + detector: DetectorImage, + computed_masks: MasksOutputs, + tutorial_centre: BeamCentre, + mask_file: Path, ) -> None: given = computed_masks.masks - outputs = bindings.beam_centre_binding.compute( - BeamCentreParams(sample=sample, masks=given) + masks = angle_mask( + detector, tutorial_centre, [AngleRange(start=0.0, stop=1.0)], base=given ) - for name, mask in given.masks.items(): - assert not np.shares_memory( - outputs.image.counts.masks[name].values, mask.values - ) + assert len(masks.masks) == 2 + assert sc.identical(masks.masks[mask_file.name], given.masks[mask_file.name]) + assert not np.shares_memory( + masks.masks[mask_file.name].values, given.masks[mask_file.name].values + ) + + +def test_a_second_angle_mask_keeps_the_ranges_of_the_first( + detector: DetectorImage, tutorial_centre: BeamCentre +) -> None: + first = AngleRange(start=0.0, stop=1.0) + second = AngleRange(start=2.0, stop=3.0) + both = angle_mask(detector, tutorial_centre, [first, second]) + chained = angle_mask( + detector, + tutorial_centre, + [second], + base=angle_mask(detector, tutorial_centre, [first]), + ) + assert chained.masks.keys() == both.masks.keys() + (name,) = both.masks + assert sc.identical(chained.masks[name], both.masks[name]) def test_iofq_is_that_of_plain_esssans( diff --git a/packages/essinstruments/tests/loki/client_test.py b/packages/essinstruments/tests/loki/client_test.py index bfd29979..9d89d10e 100644 --- a/packages/essinstruments/tests/loki/client_test.py +++ b/packages/essinstruments/tests/loki/client_test.py @@ -11,9 +11,16 @@ import pytest import scipp as sc from ess.instruments.loki.bindings import BINDINGS -from ess.instruments.loki.specs import BEAM_CENTRE, IOFQ, MASKS, BeamCentre +from ess.instruments.loki.specs import ( + BEAM_CENTRE, + CALIBRATED_DETECTOR, + CREATE_MASK, + IOFQ, + MASKS, + BeamCentre, +) -from ess.apps import Client, FolderFiles, SubmitError, Template, file, local +from ess.apps import Client, FolderFiles, Record, SubmitError, Template, file, local @pytest.fixture @@ -33,6 +40,22 @@ def client(sample: Path, mask_file: Path, direct_beam: Path) -> Iterator[Client] yield client +@pytest.fixture +def detector(client: Client) -> Record: + return client.submit(CALIBRATED_DETECTOR, params={'run': file(run=60339)}) + + +@pytest.fixture +def masks(client: Client, detector: Record, mask_file: Path) -> Record: + return client.submit( + MASKS, + params={ + 'detector': detector.ref('detector'), + 'mask_files': [file(path=mask_file)], + }, + ) + + @pytest.fixture def iofq_settings(direct_beam: Path) -> dict[str, object]: return { @@ -47,18 +70,15 @@ def iofq_settings(direct_beam: Path) -> dict[str, object]: def test_masks_then_centre_then_iofq( client: Client, - mask_file: Path, + detector: Record, + masks: Record, iofq_settings: dict[str, object], plain_esssans_iofq: sc.DataArray, tutorial_centre: BeamCentre, ) -> None: - masks = client.submit( - MASKS, - params={'run': file(run=60339), 'mask_files': [file(path=mask_file)]}, - ) centre = client.submit( BEAM_CENTRE, - params={'sample': file(run=60339), 'masks': masks.ref('masks')}, + params={'detector': detector.ref('detector'), 'masks': masks.ref('masks')}, ) iofq = client.submit( IOFQ, @@ -83,16 +103,43 @@ def test_masks_then_centre_then_iofq( assert sc.all(difference < 0.1 * sc.stddevs(plain_esssans_iofq.data)).value +def test_an_angle_mask_of_the_centre_found_changes_iofq( + client: Client, + detector: Record, + masks: Record, + iofq_settings: dict[str, object], +) -> None: + centre = client.submit( + BEAM_CENTRE, + params={'detector': detector.ref('detector'), 'masks': masks.ref('masks')}, + ) + angles = client.submit( + CREATE_MASK, + params={ + 'detector': detector.ref('detector'), + 'beam_centre': centre.ref('centre'), + 'base': masks.ref('masks'), + 'shapes': [{'start': 3.0, 'stop': 180.0}], + }, + ) + runs = {'sample': file(run=60339), 'sample_transmission': file(run=60394)} + settings = {**iofq_settings, **runs, 'beam_centre': centre.ref('centre')} + without = client.compute(IOFQ, params={**settings, 'masks': masks.ref('masks')}) + with_angles = client.compute( + IOFQ, params={**settings, 'masks': angles.ref('masks')} + ) + first = client.output(without, 'iofq').intensity + second = client.output(with_angles, 'iofq').intensity + assert sc.identical(first['Q', 0], second['Q', 0]) # well inside the range kept + assert not sc.identical(first, second) + + def test_batch_through_a_stage_with_one_can( client: Client, - mask_file: Path, + masks: Record, iofq_settings: dict[str, object], tutorial_centre: BeamCentre, ) -> None: - masks = client.submit( - MASKS, - params={'run': file(run=60339), 'mask_files': [file(path=mask_file)]}, - ) template = Template( IOFQ, params={ @@ -129,12 +176,8 @@ def test_batch_through_a_stage_with_one_can( def test_a_file_of_another_kind_is_refused( - client: Client, mask_file: Path, iofq_settings: dict[str, object] + client: Client, masks: Record, mask_file: Path, iofq_settings: dict[str, object] ) -> None: - masks = client.submit( - MASKS, - params={'run': file(run=60339), 'mask_files': [file(path=mask_file)]}, - ) with pytest.raises(SubmitError, match='does not fit the field'): client.submit( IOFQ, @@ -149,12 +192,8 @@ def test_a_file_of_another_kind_is_refused( def test_a_typed_beam_centre_without_a_distance_is_refused_at_submit( - client: Client, mask_file: Path, iofq_settings: dict[str, object] + client: Client, masks: Record, iofq_settings: dict[str, object] ) -> None: - masks = client.submit( - MASKS, - params={'run': file(run=60339), 'mask_files': [file(path=mask_file)]}, - ) with pytest.raises(SubmitError, match='beam_centre'): client.submit( IOFQ, diff --git a/packages/essinstruments/tests/loki/specs_test.py b/packages/essinstruments/tests/loki/specs_test.py index 558b3ad6..1eba4133 100644 --- a/packages/essinstruments/tests/loki/specs_test.py +++ b/packages/essinstruments/tests/loki/specs_test.py @@ -4,7 +4,7 @@ import sys import pytest -from ess.instruments.loki.specs import BeamCentre +from ess.instruments.loki.specs import AngleRange, BeamCentre from pydantic import ValidationError @@ -22,3 +22,11 @@ def test_specs_load_without_the_workflow_package() -> None: def test_a_beam_centre_without_a_distance_from_the_sample_is_refused(z: float) -> None: with pytest.raises(ValidationError, match='greater than 0'): BeamCentre(x=0.01, y=0.01, z=z) + + +@pytest.mark.parametrize(('start', 'stop'), [(2.0, 1.0), (1.0, 1.0)]) +def test_an_angle_range_that_holds_no_angle_is_refused( + start: float, stop: float +) -> None: + with pytest.raises(ValidationError, match='must exceed'): + AngleRange(start=start, stop=stop) From a5c27549a51e1978050ca20e4a9f99f191f28cd1 Mon Sep 17 00:00:00 2001 From: Simon Heybrock Date: Mon, 9 Nov 2026 09:44:40 +0000 Subject: [PATCH 2/2] loki: refuse a beam centre with no counts left, and show the detector esssans' finder raises its cutoff until pixels are left, so with every pixel masked or no counts it never returns. One typed angle range can now mask the whole detector, so the binding refuses this. Two mask files with one name no longer lose a mask without a word. The detector has its own view. A plot whose input has gone names the input and says what to do: computing it again makes a new record, which the plotted record does not reference. Reading a plot's inputs needs no widget library, so it moves next to Plot. scippnexus is imported directly, so it is declared. Adds the tests that mutating the code showed were missing, such as the angle being measured from the sample rather than the origin. Co-Authored-By: Claude Opus 5.5 --- .../essapps/src/ess/apps/panel/outputs.py | 14 +--- packages/essapps/src/ess/apps/plotting.py | 35 ++++++++- packages/essapps/tests/panel/outputs_test.py | 60 ++++++++++------ packages/essapps/tests/plotting_test.py | 21 +++++- packages/essinstruments/pyproject.toml | 1 + .../src/ess/instruments/loki/bindings.py | 13 ++-- .../src/ess/instruments/loki/plots.py | 8 +++ .../essinstruments/tests/loki/app_test.py | 8 ++- .../tests/loki/bindings_test.py | 72 +++++++++++++++++++ .../essinstruments/tests/loki/client_test.py | 30 ++++++++ .../essinstruments/tests/loki/specs_test.py | 8 +++ 11 files changed, 232 insertions(+), 38 deletions(-) diff --git a/packages/essapps/src/ess/apps/panel/outputs.py b/packages/essapps/src/ess/apps/panel/outputs.py index f400dddf..d552df3f 100644 --- a/packages/essapps/src/ess/apps/panel/outputs.py +++ b/packages/essapps/src/ess/apps/panel/outputs.py @@ -44,8 +44,8 @@ from pydantic.fields import FieldInfo from ess.apps import ArrayInfo, Client, Listing, NotReadable, Record, SpecId, Status -from ess.apps.plotting import Plot, slider_dims -from ess.spec import OutputRef, field_types +from ess.apps.plotting import Plot, read_inputs, slider_dims +from ess.spec import field_types from .graph import Graph from .table import Table, with_sidebar @@ -223,19 +223,11 @@ def _configured(self, record: Record, view: str) -> Any: outputs = model.model_construct( **{n: values.get(n) for n in model.model_fields} ) - inputs = {name: self._input(record, name) for name in plot.inputs} + inputs = read_inputs(plot, self._client, record) return _figure_pane(plot.function(outputs, **inputs)) except Exception as error: # a plot is code of the workflow's author return pn.pane.Markdown(f'{view} failed: `{type(error).__name__}: {error}`') - def _input(self, record: Record, name: str) -> Any: - """The value that the param ``name`` of the record's request references.""" - ref = record.request.params[name] - if not isinstance(ref, OutputRef) or ref.key is not None: - raise ValueError(f'{name} does not reference a whole output') - (source,) = self._client.records(ids=[ref.record]) - return self._client.output(source, ref.output) - def _plot(self, record: Record, name: str) -> list[Any]: """ A figure for each array field of the output; a field of more than two diff --git a/packages/essapps/src/ess/apps/plotting.py b/packages/essapps/src/ess/apps/plotting.py index 5c3f38ea..2a9c3360 100644 --- a/packages/essapps/src/ess/apps/plotting.py +++ b/packages/essapps/src/ess/apps/plotting.py @@ -16,6 +16,12 @@ from dataclasses import dataclass from typing import Any +from ess.spec import OutputRef + +from .client import Client +from .errors import NotReadable +from .records import Record + @dataclass(frozen=True) class Plot: @@ -30,13 +36,40 @@ class Plot: Plot(masks_on_image, inputs=('detector',)) - An input that is no longer readable fails the plot. """ function: Callable[..., Any] inputs: tuple[str, ...] = () +def read_inputs(plot: Plot, client: Client, record: Record) -> dict[str, Any]: + """ + The value of each input of ``plot`` that the request of ``record`` references. + + Raises + ------ + ValueError + If a param holds anything but a reference to a whole output. + NotReadable + If the output is no longer readable, which names the param: computing the + output again makes a new record, which this record does not reference. + """ + values = {} + for name in plot.inputs: + ref = record.request.params[name] + if not isinstance(ref, OutputRef) or ref.key is not None: + raise ValueError(f'{name} does not reference a whole output') + (source,) = client.records(ids=[ref.record]) + try: + values[name] = client.output(source, ref.output) + except NotReadable as error: + raise NotReadable( + f'{name} is no longer available. Compute it again, and this ' + f'record from the result ({error})' + ) from error + return values + + def slider_dims(sizes: Mapping[str, int]) -> list[str]: """ The dimensions of an array that get a slider: all but the last two, which diff --git a/packages/essapps/tests/panel/outputs_test.py b/packages/essapps/tests/panel/outputs_test.py index 4d01b14a..7e251767 100644 --- a/packages/essapps/tests/panel/outputs_test.py +++ b/packages/essapps/tests/panel/outputs_test.py @@ -105,6 +105,10 @@ class ScaleOutputs(BaseModel): total: Total +class FrameParams(BaseModel): + picture: Referenced[Image] + + CURVE = WorkflowSpec( name='curve', version=1, @@ -123,6 +127,16 @@ class ScaleOutputs(BaseModel): ) +FRAME = WorkflowSpec( + name='frame', + version=1, + title='Frame', + description='The total of an image.', + params=FrameParams, + outputs=ScaleOutputs, +) + + CUBE = WorkflowSpec( name='cube', version=1, @@ -186,9 +200,13 @@ def curve(params: CurveParams) -> CurveOutputs: def scale(params: ScaleParams) -> ScaleOutputs: return ScaleOutputs(total=Total(value=params.curve.total)) + def frame(params: FrameParams) -> ScaleOutputs: + return ScaleOutputs(total=Total(value=params.picture.counts.sum())) + bind = [ function_binding(CURVE, curve), function_binding(SCALE, scale), + function_binding(FRAME, frame), function_binding(CUBE, cube), ] with local(proposal='p1', files=run[0], bind=bind) as client: @@ -666,45 +684,47 @@ def test_a_configured_plot_that_fails_shows_why_as_text( assert 'RuntimeError: no plot today' in text.object -def scale_on_curve(out: ScaleOutputs, curve: Curve) -> Any: - return pp.plot(curve.counts, title=f'{out.total.__class__.__name__} on curve') +def draw_picture(out: ScaleOutputs, *, picture: Image) -> Any: + return pp.plot(picture.counts, title=str(picture.counts.sum().value)) @pytest.fixture -def scale_view(client: Client, file: FileRef) -> tuple[Outputs, Record, Record]: - """The outputs with a view of a scale that draws the curve it referenced.""" +def frame_view(client: Client, file: FileRef) -> tuple[Outputs, Record, Record]: + """The outputs with a view of a frame that draws the image it referenced.""" curve = client.submit(CURVE, params={'run': file}) - scale = client.submit(SCALE, params={'curve': curve.ref('curve')}) - client.wait([curve, scale]) - plots = {SpecId.of(SCALE): {'On curve': Plot(scale_on_curve, inputs=('curve',))}} - return Outputs(client, Listing(client), plots=plots), curve, scale + frame = client.submit(FRAME, params={'picture': curve.ref('image')}) + client.wait([curve, frame]) + plots = {SpecId.of(FRAME): {'Picture': Plot(draw_picture, inputs=('picture',))}} + return Outputs(client, Listing(client), plots=plots), curve, frame def test_a_configured_plot_takes_the_value_its_record_referenced( - scale_view: tuple[Outputs, Record, Record], + client: Client, frame_view: tuple[Outputs, Record, Record] ) -> None: - outputs, _, scale = scale_view - outputs.select(scale, 'total') + outputs, curve, frame = frame_view + outputs.select(frame, 'total') - outputs.views.value = 'On curve' + outputs.views.value = 'Picture' (pane,) = outputs.view.objects assert isinstance(pane, pn.pane.Matplotlib), pane.object - assert pane.object.axes[0].get_title() == 'Total on curve' + total = client.output(curve, 'image').counts.sum().value + assert pane.object.axes[0].get_title() == str(total) -def test_a_configured_plot_whose_input_is_released_shows_why_as_text( - client: Client, scale_view: tuple[Outputs, Record, Record] +def test_a_configured_plot_whose_input_is_released_says_what_to_do( + client: Client, frame_view: tuple[Outputs, Record, Record] ) -> None: - outputs, curve, scale = scale_view + outputs, curve, frame = frame_view client.release(curve) - outputs.select(scale, 'total') + outputs.select(frame, 'total') - outputs.views.value = 'On curve' + outputs.views.value = 'Picture' (text,) = outputs.view.objects - assert 'On curve failed' in text.object - assert 'NotReadable' in text.object + assert 'Picture failed' in text.object + assert 'NotReadable: picture is no longer available' in text.object + assert 'Compute it again' in text.object def test_a_configured_plot_input_must_reference_an_output( diff --git a/packages/essapps/tests/plotting_test.py b/packages/essapps/tests/plotting_test.py index c2673b15..a83ecfb6 100644 --- a/packages/essapps/tests/plotting_test.py +++ b/packages/essapps/tests/plotting_test.py @@ -4,8 +4,13 @@ import subprocess import sys +from datetime import UTC, datetime -from ess.apps.plotting import slider_dims +import pytest + +from ess.apps import Record, Request, SpecId +from ess.apps.plotting import Plot, read_inputs, slider_dims +from ess.spec import OutputRef def test_every_dimension_but_the_last_two_has_a_slider() -> None: @@ -19,6 +24,20 @@ def test_every_dimension_but_the_last_two_has_a_slider() -> None: assert slider_dims({}) == [] +def test_a_plot_input_cannot_be_an_element_of_an_output() -> None: + element = OutputRef(record='r', output='images', key='first') + record = Record( + id='s', + request=Request(SpecId(name='s', version=1), params={'image': element}), + proposal='p', + submitter='u', + created=datetime.now(UTC), + outputs=(), + ) + with pytest.raises(ValueError, match='image does not reference a whole output'): + read_inputs(Plot(print, inputs=('image',)), None, record) # type: ignore[arg-type] + + def test_what_an_application_decides_about_plots_needs_no_panel() -> None: code = ( 'import sys; import ess.apps, ess.apps.plotting, ess.apps.listing;' diff --git a/packages/essinstruments/pyproject.toml b/packages/essinstruments/pyproject.toml index 598bb585..ce2c3db2 100644 --- a/packages/essinstruments/pyproject.toml +++ b/packages/essinstruments/pyproject.toml @@ -35,6 +35,7 @@ dependencies = [ loki = [ "esssans>=26.10.0", "essspec[sciline]", + "scippnexus>=25.06.0", ] loki-app = [ "essinstruments[loki]", diff --git a/packages/essinstruments/src/ess/instruments/loki/bindings.py b/packages/essinstruments/src/ess/instruments/loki/bindings.py index 8353c799..55e73d00 100644 --- a/packages/essinstruments/src/ess/instruments/loki/bindings.py +++ b/packages/essinstruments/src/ess/instruments/loki/bindings.py @@ -105,6 +105,10 @@ def load_detector(params: CalibratedDetectorParams) -> CalibratedDetectorOutputs def compute_masks(params: MasksParams) -> MasksOutputs: + names = [path.name for path in params.mask_files] + if len(set(names)) < len(names): + # Masks are named by file, so that one mask would replace the other. + raise ValueError(f'mask files with the same name: {sorted(names)}') masks: dict[str, sc.Variable] = {} if params.mask_files: workflow = LokiAtLarmorWorkflow() @@ -118,14 +122,15 @@ def compute_masks(params: MasksParams) -> MasksOutputs: def find_beam_centre(params: BeamCentreParams) -> BeamCentreOutputs: - counts = params.detector.counts + counts = params.detector.counts.assign_masks(params.masks.masks) + # The finder raises its cutoff until pixels are left, so without any it never ends. + if not counts.sum().value > 0: + raise ValueError('no counts outside the masks to find a beam centre from') workflow = LokiAtLarmorWorkflow() workflow[Position[snx.NXsample, SampleRun]] = counts.coords['sample_position'] workflow[Position[snx.NXsource, SampleRun]] = counts.coords['source_position'] # The finder sums the dimensions that the positions lack, and else expects events. - workflow[CorrectedDetector[SampleRun, Numerator]] = sc.concat( - [counts.assign_masks(params.masks.masks)], 'summed' - ) + workflow[CorrectedDetector[SampleRun, Numerator]] = sc.concat([counts], 'summed') centre = sans.beam_center_from_center_of_mass(workflow).to(unit='m') return BeamCentreOutputs( centre=BeamCentre( diff --git a/packages/essinstruments/src/ess/instruments/loki/plots.py b/packages/essinstruments/src/ess/instruments/loki/plots.py index ed8f5112..03fac958 100644 --- a/packages/essinstruments/src/ess/instruments/loki/plots.py +++ b/packages/essinstruments/src/ess/instruments/loki/plots.py @@ -16,10 +16,12 @@ from .specs import ( BEAM_CENTRE, + CALIBRATED_DETECTOR, CREATE_MASK, IOFQ, MASKS, BeamCentreOutputs, + CalibratedDetectorOutputs, DetectorImage, IofQOutputs, MasksOutputs, @@ -37,6 +39,11 @@ def on_detector(image: sc.DataArray) -> sc.DataArray: return pixels.drop_coords(others).hist(y=128, x=128) +def detector_image(outputs: CalibratedDetectorOutputs) -> Figure: + """The counts of every pixel.""" + return pp.plot(on_detector(outputs.detector.counts), norm='log', title='All pixels') + + def masks_on_image(outputs: MasksOutputs, detector: DetectorImage) -> Figure: """The detector image without the masked pixels, which are left blank.""" return pp.plot( @@ -74,6 +81,7 @@ def transmission_curves(outputs: IofQOutputs) -> Figure: _MASKS_ON_IMAGE = Plot(masks_on_image, inputs=('detector',)) PLOTS = { + SpecId.of(CALIBRATED_DETECTOR): {'Detector image': Plot(detector_image)}, SpecId.of(MASKS): {'Masks on image': _MASKS_ON_IMAGE}, SpecId.of(CREATE_MASK): {'Masks on image': _MASKS_ON_IMAGE}, SpecId.of(BEAM_CENTRE): { diff --git a/packages/essinstruments/tests/loki/app_test.py b/packages/essinstruments/tests/loki/app_test.py index 1543652f..d1c90d6c 100644 --- a/packages/essinstruments/tests/loki/app_test.py +++ b/packages/essinstruments/tests/loki/app_test.py @@ -116,6 +116,7 @@ def test_the_plots_of_loki_draw_the_outputs_of_real_records(session: Session) -> outputs = session.app.outputs drawn = {} for record, name, view in [ + (session.detector, 'detector', 'Detector image'), (session.masks, 'masks', 'Masks on image'), (session.angles, 'masks', 'Masks on image'), (session.centre, 'centre', 'Centre on image'), @@ -136,8 +137,13 @@ def of(record: Record, view: str) -> Any: for record in (session.masks, session.angles) ) assert 0 < angle_masks < file_masks + all_pixels = of(session.detector, 'Detector image').images[0].get_array() + assert file_masks < np.nansum(all_pixels) centre = of(session.centre, 'Centre on image') - assert any(line.get_marker() == '+' for line in centre.lines) + assert np.nansum(centre.images[0].get_array()) == file_masks + (cross,) = (line for line in centre.lines if line.get_marker() == '+') + found = session.app.client.output(session.centre, 'centre') + assert (cross.get_xdata()[0], cross.get_ydata()[0]) == (found.x, found.y) assert of(session.iofq, 'I(Q)').get_yscale() == 'log' legend = of(session.iofq, 'Transmission').get_legend() assert [t.get_text() for t in legend.get_texts()] == ['sample', 'can'] diff --git a/packages/essinstruments/tests/loki/bindings_test.py b/packages/essinstruments/tests/loki/bindings_test.py index cc468b77..33556393 100644 --- a/packages/essinstruments/tests/loki/bindings_test.py +++ b/packages/essinstruments/tests/loki/bindings_test.py @@ -4,6 +4,7 @@ The bindings on the runs of the LoKI detector test, against plain esssans. """ +import shutil from pathlib import Path import numpy as np @@ -64,6 +65,29 @@ def test_masks_are_named_by_file_and_fit_the_detector( assert detector.counts.sizes == masks[mask_file.name].sizes +def test_each_mask_file_gives_a_mask( + detector: DetectorImage, mask_file: Path, tmp_path: Path +) -> None: + copy = tmp_path / 'copy.xml' + shutil.copy(mask_file, copy) + outputs = bindings.masks_binding.compute( + MasksParams(detector=detector, mask_files=[mask_file, copy]) + ) + assert set(outputs.masks.masks) == {mask_file.name, 'copy.xml'} + + +def test_mask_files_with_the_same_name_are_refused( + detector: DetectorImage, mask_file: Path, tmp_path: Path +) -> None: + (tmp_path / 'a').mkdir() + (tmp_path / 'b').mkdir() + files = [tmp_path / 'a' / 'm.xml', tmp_path / 'b' / 'm.xml'] + for path in files: + shutil.copy(mask_file, path) + with pytest.raises(ValueError, match='the same name'): + bindings.masks_binding.compute(MasksParams(detector=detector, mask_files=files)) + + def test_without_mask_files_no_pixel_is_masked(detector: DetectorImage) -> None: outputs = bindings.masks_binding.compute( MasksParams(detector=detector, mask_files=[]) @@ -85,6 +109,18 @@ def test_beam_centre_is_that_of_the_tutorial( assert outputs.centre.z == pytest.approx(tutorial_centre.z, abs=1e-5) +def test_a_beam_centre_without_counts_outside_the_masks_is_refused( + detector: DetectorImage, +) -> None: + everything = sc.ones(sizes=detector.counts.sizes, dtype=bool) + with pytest.raises(ValueError, match='no counts outside the masks'): + bindings.beam_centre_binding.compute( + BeamCentreParams( + detector=detector, masks=PixelMasks(masks={'all': everything}) + ) + ) + + def angle_mask( detector: DetectorImage, centre: BeamCentre, @@ -122,6 +158,42 @@ def test_an_angle_range_masks_the_pixels_at_those_angles( ) +def two_pixels(sample_z: float, *positions: list[float]) -> DetectorImage: + pixels = sc.vectors(dims=['pixel'], values=positions, unit='m') + return DetectorImage( + counts=sc.DataArray( + sc.ones(sizes=pixels.sizes), + coords={ + 'position': pixels, + 'sample_position': sc.vector([0.0, 0.0, sample_z], unit='m'), + }, + ) + ) + + +def test_an_angle_is_measured_from_the_sample() -> None: + # 0 and 90 degrees from the sample at z = 1, but 0 and 45 from the origin. + detector = two_pixels(1.0, [0.0, 0.0, 2.0], [1.0, 0.0, 1.0]) + masks = angle_mask( + detector, + BeamCentre(x=0.0, y=0.0, z=1.0), + [AngleRange(start=0.0, stop=60.0)], + ) + (mask,) = masks.masks.values() + assert mask.values.tolist() == [True, False] + + +def test_an_angle_range_holds_its_start_but_not_its_stop() -> None: + detector = two_pixels(0.0, [0.0, 0.0, 1.0], [1.0, 0.0, 0.0]) + masks = angle_mask( + detector, + BeamCentre(x=0.0, y=0.0, z=1.0), + [AngleRange(start=0.0, stop=90.0)], + ) + (mask,) = masks.masks.values() + assert mask.values.tolist() == [True, False] + + def test_an_angle_is_measured_from_the_beam_centre( detector: DetectorImage, tutorial_centre: BeamCentre ) -> None: diff --git a/packages/essinstruments/tests/loki/client_test.py b/packages/essinstruments/tests/loki/client_test.py index 9d89d10e..9b6b903e 100644 --- a/packages/essinstruments/tests/loki/client_test.py +++ b/packages/essinstruments/tests/loki/client_test.py @@ -134,6 +134,36 @@ def test_an_angle_mask_of_the_centre_found_changes_iofq( assert not sc.identical(first, second) +def test_a_mask_around_a_typed_centre_needs_no_other_masks( + client: Client, detector: Record +) -> None: + angles = client.compute( + CREATE_MASK, + params={ + 'detector': detector.ref('detector'), + 'beam_centre': {'x': 0.0, 'y': 0.0, 'z': 1.0}, + 'base': None, + 'shapes': [{'start': 0.0, 'stop': 1.0}], + }, + ) + assert client.failure(angles) is None + + +def test_a_mask_without_ranges_is_refused_at_submit( + client: Client, detector: Record +) -> None: + with pytest.raises(SubmitError, match='shapes'): + client.submit( + CREATE_MASK, + params={ + 'detector': detector.ref('detector'), + 'beam_centre': {'x': 0.0, 'y': 0.0, 'z': 1.0}, + 'base': None, + 'shapes': [], + }, + ) + + def test_batch_through_a_stage_with_one_can( client: Client, masks: Record, diff --git a/packages/essinstruments/tests/loki/specs_test.py b/packages/essinstruments/tests/loki/specs_test.py index 1eba4133..34866836 100644 --- a/packages/essinstruments/tests/loki/specs_test.py +++ b/packages/essinstruments/tests/loki/specs_test.py @@ -24,6 +24,14 @@ def test_a_beam_centre_without_a_distance_from_the_sample_is_refused(z: float) - BeamCentre(x=0.01, y=0.01, z=z) +@pytest.mark.parametrize(('start', 'stop'), [(-1.0, 1.0), (1.0, 181.0)]) +def test_an_angle_range_outside_0_to_180_degrees_is_refused( + start: float, stop: float +) -> None: + with pytest.raises(ValidationError): + AngleRange(start=start, stop=stop) + + @pytest.mark.parametrize(('start', 'stop'), [(2.0, 1.0), (1.0, 1.0)]) def test_an_angle_range_that_holds_no_angle_is_refused( start: float, stop: float