Add the BIFROST elastic monitor Q-map - #1236
Draft
SimonHeybrock wants to merge 9 commits into
Draft
Conversation
The Bragg peak monitor is the elastic monitor (cbm5), a single-pixel monitor mounted on the detector tank. Over a sample rotation scan it maps elastic intensity in Q, which the scientists want binned in Q perpendicular vs Q parallel across the full wavelength band. Register it against the monitor source rather than unified_detector, so this is the first reduction spec whose primary source is a monitor. The two rotation angles it needs are declared at spec scope, because the instrument-scope bindings are tied to unified_detector. Accumulate the map in the backend rather than emitting a timeseries for the frontend correlation histogram: BIFROST runs for days, and the histogram costs memory proportional to the bin count instead of the run length. The frontend's full-history buffer is a 20 MB ring that discards the start of the run. The end-to-end test is a strict xfail. Routing, gating on a3/a4, and job activation all work; what does not is that the monitor geometry read from the geometry file carries a 'time' dimension from that file's detector-tank-angle log, while streamed events carry 'event_time_zero'. The position needs to come from the live InstrumentAngle context instead. Requires the essspectroscopy change on scipp/ess branch bragg-peak-monitor-nxmonitor, which sources this monitor's geometry from the NXmonitor it actually is.
The workflow is not blocked on deriving the monitor position from the live instrument angle, as the previous xfail claimed. A4 already arrives as a context stream. What the geometry file must supply is only the monitor's static position in the tank frame. Verified by grafting the previous survey's monitor chain into an artifact regenerated from coda_bifrost_999999_00016610.hdf and terminating it before the tank rotation: the transformation then evaluates to a scalar, EmptyDetector carries a scalar position, and the end-to-end test passes, producing a 2D map from streamed monitor events. Restated the xfail accordingly. Two things still stand in the way of a shippable artifact. The producer stopped writing transformations for the two event-mode monitors, so their depends_on dangles. And chains that route through detector_tank_angle cannot be evaluated from an artifact at all, because the generation script trims that NXlog to length zero -- the pinned artifact's detector chains already stop short of it.
Records what the scientists asked for, why the map is accumulated in the backend rather than assembled by the frontend correlation histogram, what landed on both branches, and the one remaining blocker. Restates the xfail: the chain runs through detector_tank_angle, whose value is live, so it needs a chain-patch context binding rather than a geometry file that stops short of the tank. The previous wording claimed the artifact must truncate there, which is wrong in general -- depends_on runs toward the root, so truncating would discard static transforms beyond the dynamic node. Also captures the dead ends, so the next session does not retrace them, and the producer-side regression that blocks regenerating the artifact.
The Bragg peak monitor rides the detector tank, so its position is only known once the live tank-angle readback arrives. Bind that readback as a chain patch (ADR 0003) rather than as a direct context value. Geometry moves off the McStas simulation file onto the geometry artifact. The patch target is the f144 stream's nexus_path, and only the artifact writes the chain entry there; the simulation file keys the same transform one level up and stores a 720-sample rotation scan in it, which no live readback can replace. Nothing else held the workflow to the simulation file. The tank angle is needed twice, as geometry and as the coordinate group_by_rotation bins on, but a stream carries one context key per spec. The chain patch is the binding and a provider derives the coordinate from the same log. The plain monitor histogram opts out of instrument contexts: it is counts-over-TOA and must not wait on the motor readback. Verified end-to-end against an artifact with the monitor's transformations restored: streamed monitor events yield a populated (Q_perpendicular, Q_parallel) map driven by the live angle. The registered artifact still lacks those transformations because the writer omits them for event-mode monitors, which three strict markers now record; fixing the artifact clears all three at once. Also lets the chain walker in motion_binding_test handle sources that are not NeXus groups, which BIFROST's logical unified_detector is not.
The monitor transformations are not missing from the new geometry file. The writer moved them to sibling groups, `elastic_monitor_backup` and `normalization_monitor_da00`, both typed NXnote, leaving the NXmonitor group with only its event data and a depends_on pointing at a `transformations` group it no longer has. make_geometry_nexus.py copies those siblings through, so the artifact already carries the correct transformations under the wrong parent, with values bit-identical to the June file (all 786 scalar transforms compare equal across the two) and the tank-angle link already on the new `/value` convention. Repairing the artifact is therefore a copy within one file, verified to yield the same populated map. Records the repair recipe and why it must not go into make_geometry_nexus.py, which copies its source faithfully.
`geometry-bifrost-repaired-2026-08-11.nxs` is the regeneration from `coda_bifrost_999999_00016610.hdf` with the two event-mode monitors' transformations copied from the `_backup`/`_da00` NXnote siblings the writer put them on, onto the NXmonitor groups their depends_on already points at. Six added datasets, no other byte changed; uploaded to the geometry-v0 release. That completes the Bragg peak monitor Q-map: the monitor's chain now resolves and terminates at the tank-angle placeholder the live readback is patched into, so the three strict markers that recorded the blockage all go. BIFROST also gets resolvable detector chains for the first time, the older artifacts having died on a stale `117_detector_tank_angle` reference. Nothing downstream depends on them: the detector views are logical and the Q-cut workflows read the McStas file. The name records that this is a hand-patched stopgap, to be superseded once the writer attaches monitor geometry to the NXmonitor group.
This was referenced Aug 11, 2026
Follows 591151e, which renamed cbm5 bragg_peak_monitor -> elastic_monitor, and the scientists' request in #555. The workflow chooser lists bare titles within a group, so the title keeps a "(monitor)" qualifier to stay distinguishable from the elastic_qmap detector workflow. Upstream still exposes BifrostBraggPeakMonitorWorkflow; the spec comment records the correspondence so the name is not a surprise at the call site.
The doc was status-shaped -- what is done, what is next -- and its durable content now lives where it is read: the chain-patch reasoning and the choice of geometry source are already comments in factories.py, the backend accumulation rationale joins the factory, and what "repaired" means in the BIFROST artifact name joins the registry entry that would otherwise leave a reader guessing. The remaining items are follow-ups rather than documentation: the writer bug, the two-geometry-file split, and the #962 pin decision.
No registered artifact produces a dangling chain any longer -- the repaired BIFROST file was the last one -- so the suppression only stood between a future regression and its own warning. The KeyError guard stays: it covers logical source names such as BIFROST's unified_detector, which is not a NeXus group at all.
SimonHeybrock
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds the BIFROST elastic monitor Q-map: over a sample rotation scan, the elastic monitor (cbm5) accumulates a 2D map of elastic intensity in Q, to be compared against the expected reciprocal lattice. Requested by the scientists; explicitly not commissioning-critical.
Named per the scientists' request in #555 ("Bragg-peak-monitor Q-map" -> "Elastic Q-map"), following
591151ebf, which renamed cbm5bragg_peak_monitor->elastic_monitor. The workflow chooser lists bare titles within a group, so the title carries a(monitor)qualifier to stay distinguishable from theelastic_qmapdetector workflow. Upstream still calls itBifrostBraggPeakMonitorWorkflow; a rename there is a separate PR.Supersedes #555's branch, which is not worth rebasing — its
exclude_from_mergeplumbing exists only because it assumed the monitor arrived on the detector topic, and cbm5 is anNXmonitoronbifrost_beam_monitor.Blocked on scipp/ess#698, which makes the workflow load the monitor as an
NXmonitor. Against released essspectroscopy exactly two tests fail — the new service test and the workflow roundtrip forbifrost/elastic_monitor_qmap/1— both because the released code routes the monitor through the detector/analyzer path. Draft until that lands and is released.Why the map is accumulated in the backend
The alternative was to emit a scalar timeseries and let the dashboard's
correlation_histogram_2dbuild the map, which is what #425 asked for and whatmonitor_histogram'stotal_in_rangeview already emits. That route is right for alignment scans and wrong here. The requirement is the full wavelength band, so each update carries events spanning many Q — a distribution, not a scalar. And BIFROST runs for days: a histogram costs memory proportional to bin count, a timeseries proportional to run length, andFullHistoryExtractorwould fall back to ring-buffer eviction against a 20 MB budget, silently discarding the start of the run.The tank angle, twice
The monitor rides the detector tank, so its
depends_onchain runs throughdetector_tank_angleand its position is only known once the live readback arrives. That is a chain-patch binding (ADR 0003). Three things fell out of it:Geometry moves off the McStas simulation file onto the geometry artifact. The patch target is the f144 stream's
nexus_path, and only the artifact writes the chain entry there; the simulation file keys the same transform one level up and stores a 720-sample rotation scan in it, which no live readback can replace. Nothing else held the workflow to the simulation file.The tank angle is needed both as geometry and as the coordinate
group_by_rotationbins on, but a stream carries one context key per spec. So the chain patch is the binding and a provider derives the coordinate from the same log.The plain monitor histogram opts out of instrument contexts: it is counts-over-TOA and must not wait on the motor readback.
Geometry artifact
Registers
geometry-bifrost-repaired-2026-08-11.nxs. The BIFROST writer attaches the event-mode monitors' geometry to<name>_backup/<name>_da00NXnote siblings while theNXmonitor'sdepends_onstill points at its own, absent,transformations. The artifact is the regeneration fromcoda_bifrost_999999_00016610.hdfwith those transformations copied onto the monitors they belong to — six added datasets, no other byte changed, values bit-identical to the June file. The name records that it is a hand-patched stopgap; it should be superseded once the writer is fixed.This also gives BIFROST resolvable detector
depends_onchains for the first time, older artifacts having died on a stale117_detector_tank_anglereference. Nothing downstream depends on them: the detector views are logical and the Q-cut workflows read the McStas file.Testing
data_reductionand asserts a populated(Q_perpendicular, Q_parallel)map (1952 of 2000 events land in it).LIVEDATA_DATA_DIRbefore upload.