[ESSSPECTROSCOPY] Source the BIFROST Bragg peak monitor from the NXmonitor - #698
Open
SimonHeybrock wants to merge 3 commits into
Open
[ESSSPECTROSCOPY] Source the BIFROST Bragg peak monitor from the NXmonitor#698SimonHeybrock wants to merge 3 commits into
SimonHeybrock wants to merge 3 commits into
Conversation
The Bragg peak monitor is BIFROST's elastic monitor (cbm5), written as an
NXmonitor in CODA files, in the pinned geometry artifacts, and in the McStas
simulation file alike. The single-crystal workflow instead required it as
NeXusComponent[NXdetector], so loading raised
ValueError: The NeXus group 'elastic_monitor' was expected to be a
NXdetector but is a NXmonitor
against every real file. Only the user guide worked, by standing a detector
triplet in for the monitor.
Take the component, its transformation and its event data from ElasticMonitor,
which the workflow already declares in monitor_types, so all the loading nodes
exist. A monitor has no pixel offsets, so the position is the transformed
origin as in get_calibrated_monitor rather than compute_detector_position, and
no detector_number, so the single pixel is named explicitly for event assembly.
The Analyzer dependency is dropped: get_base_calibrated_detector_bifrost never
reads it, and a monitor in the direct beam has no analyzer.
Ltotal is a straight line for the same reason, so insert that provider rather
than leaving every caller to supply it.
Fix a latent defect this exposes: a time-dependent position makes ltotal depend
on 'time', but group_by_rotation has already renamed that same dimension to
'a4', so the broadcast in detector_wavelength_data rejected it. This bites
whenever the tank rotates; the user guide escaped it only because its stand-in
position is static.
The simulated data contains no Bragg peak monitor, so the detector stand-in
moves to simulation_providers and keeps the user guide working.
Streamed monitor events carry no event_id, so assemble_detector_data cannot group them. The monitor is a single pixel, so assign its geometry onto the events as assemble_monitor_data does. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
3 tasks
github-actions
Bot
changed the title
The reason for not using assemble_detector_data is not that streamed events lack an event_id. It is that assemble_detector_data groups events by event_id onto a detector_number grid, and the Bragg peak monitor is a single pixel with nothing to group by. The detector_number assigned to it existed only to satisfy that grouping and is now unused. Call assemble_monitor_data, which is exactly the remaining operation: assign the geometry onto the events. This drops the copy of its body and the private _add_variances import. The result is independent of whether the events carry an event_id, so one provider serves a file-loaded cbm5_events group (which has one, constant 1) and a stream (which, in esslivedata, does not: its adapter discards the ev44 pixel_id for monitors not registered as pixellated). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
get_calibrated_bragg_peak_detectorrequiredNeXusComponent[NXdetector], but the Bragg peak monitor is the elastic monitor (cbm5), written as anNXmonitorin CODA files, the geometry artifacts and the McStas simulation file alike. Loading therefore raisedagainst every real file. Only the user guide worked, by standing a detector triplet in for the monitor.
The monitor's component, transformation and event data now come from
ElasticMonitor, which the workflow already declares inmonitor_types— a re-pointing rather than new machinery. A monitor has no pixel offsets, so the position is the transformed origin as inget_calibrated_monitor.Analyzeris dropped:get_base_calibrated_detector_bifrostnever reads it, and a monitor in the direct beam has none. The detector stand-in moves tosimulation_providers, keeping the user guide working.Assembly likewise uses
assemble_monitor_data.assemble_detector_datagroups events byevent_idonto adetector_numbergrid, and a single-pixel monitor has nothing to group by; what remains is the geometry assignmentassemble_monitor_dataalready does. This holds whether or not the events carry anevent_id— a file-loadedcbm5_eventsgroup does, constant1throughout; a stream may not.Also fixes a latent defect that is not monitor-specific: a time-dependent position makes
ltotalcarrytime, butgroup_by_rotationhas already renamed that dimension toa4, so the broadcast indetector_wavelength_datarejected it. This bites whenever the tank rotates. It stayed hidden because the user guide's stand-in position is static and the inelastic/Q-cut path uses adetector_wavelength_datathat takes noltotal.Testing
No regression test for the monitor path here — the simulation file's
elastic_monitoris histogram-mode, so there is no simulated Bragg peak monitor to drive. It is covered end to end in scipp/esslivedata#1236, where streamed monitor events and live rotation readbacks produce a populated (Qpar, Qperp) map over a multi-angle scan. That branch needs this PR, so it is blocked on it.Open question
The scientists asked for the map in the laboratory frame, but
project_momentum_transferprojectssample_table_momentum_transfer— the sample-table frame, which is what you want against an expected reciprocal lattice. Worth settling before merge.