From 25d25cff87f030bb1b0a5e8d881ddc8f12f5143b Mon Sep 17 00:00:00 2001 From: Gregory Tucker Date: Fri, 27 Feb 2026 10:21:31 +0100 Subject: [PATCH 1/2] [Fix] Remove time-dependence from geometry calcs The positions and orientation of components after the sample position are, strictly speaking, time-dependent parameters. Previously their transformation chains lacked this fidelity due to limitations in creating the NeXus Structure JSON via `moreniius`. Those limitations have been lifted and newly-generated BIFROST simulation files include NXtransformation groups with NXlog constituents. In order to calculate the correct analyzer and sample scattering angle for each detector pixel, the BIFROST workflow performs some geometry calculations in the coordinate frame which rotates with the detector tank, around the sample position. Since the analyzers and detectors are stationary in this frame, we can (and must) remove any time-dependence from their positions and orientations. This PR introduces a quick-and-dirty hack to only use the first time-point from any time-dependent transformation result for the - sample - analyzers - detectors --- src/ess/spectroscopy/indirect/kf.py | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/src/ess/spectroscopy/indirect/kf.py b/src/ess/spectroscopy/indirect/kf.py index e62d09a..54f03e4 100644 --- a/src/ess/spectroscopy/indirect/kf.py +++ b/src/ess/spectroscopy/indirect/kf.py @@ -19,6 +19,14 @@ ) +def _no_time(x: sc.Variable | sc.DataArray) -> sc.Variable: + if isinstance(x, sc.DataArray) and 'time' in x.coords: + return x['time', 0].data + elif isinstance(x, sc.DataArray): + raise ValueError("Only `DataArray`s with a time-coordinate allowed") + return x + + def sample_analyzer_vector( sample_position: sc.Variable, analyzer_position: sc.Variable, @@ -52,6 +60,17 @@ def sample_analyzer_vector( The vector from the sample position to the interaction point on the analyzer for each detector element. """ + # FIXME time-dependent depends-on chains produce positions and transformations + # which are `scipp.DataArray`s with a 'time' coordinate. We don't need the + # time-dependence here since all calculations are done in the rotating + # detector-tank coordinate system where these *have no* time-dependence + # TODO: Verify that we are actually using the rotating detector-tank coordinate + # frame, otherwise we will misidentify the Q vector for each detector + sample_position = _no_time(sample_position) + analyzer_position = _no_time(analyzer_position) + analyzer_transform = _no_time(analyzer_transform) + detector_position = _no_time(detector_position) + # Scipp does not distinguish between coordinates and directions, so we need to do # some extra legwork to ensure we can apply the orientation transformation # _and_ obtain a dimensionless direction vector @@ -123,6 +142,16 @@ def analyzer_detector_vector( detector_position: sc.Variable, ) -> sc.Variable: """Calculate the analyzer-detector vector""" + # FIXME time-dependent depends-on chains produce positions and transformations + # which are `scipp.DataArray`s with a 'time' coordinate. We don't need the + # time-dependence here since all calculations are done in the rotating + # detector-tank coordinate system where these *have no* time-dependence + # TODO: Verify that we are actually using the rotating detector-tank coordinate + # frame, otherwise we will misidentify the Q vector for each detector + sample_position = _no_time(sample_position) + sample_analyzer_vector = _no_time(sample_analyzer_vector) # FIXME unnecessary? + detector_position = _no_time(detector_position) + analyzer_position = sample_position + sample_analyzer_vector.to( unit=sample_analyzer_vector.unit ) From c0aa844c47d6fb11367673947350ef9c8451d80e Mon Sep 17 00:00:00 2001 From: Gregory Tucker Date: Fri, 27 Feb 2026 11:19:08 +0100 Subject: [PATCH 2/2] [Add] @inputs for detector->analyzer resolution The NeXus format specification now allows most base objects to specify two attributes which help define the *expected* beam path through a collection of instrument components: `@inputs` and `@outputs`. Updates to `moreniius` mean that the BIFROST NeXus Structure JSON now contains these attributes, including the one-to-many and many-to-one transitions between, e.g., the sample position and the 45 analyzers. Given a detector's HDF5 Group, it is possible to follow the @inputs attributes analagously to a depends-on chain to identify which analyzer the detector should have received its neutron events from. This commit adds functionality which has only been tested outside of the workflow, by defining the `DetectorAnalyzerMap` at repl scope and the patching `ess.bifrost.io.nexus._get_analyzer_for_detector_name` to use the static relational information. I imagine it needs work to mold it into an appropriate `sciline` workflow. --- src/ess/bifrost/io/nexus.py | 93 +++++++++++++++++++++++++++++++++---- src/ess/bifrost/types.py | 3 ++ 2 files changed, 86 insertions(+), 10 deletions(-) diff --git a/src/ess/bifrost/io/nexus.py b/src/ess/bifrost/io/nexus.py index 41b3297..25cd0ab 100644 --- a/src/ess/bifrost/io/nexus.py +++ b/src/ess/bifrost/io/nexus.py @@ -6,6 +6,7 @@ import scipp as sc import scippnexus as snx +from ess.bifrost.types import DetectorAnalyzerMap from ess.reduce.nexus import load_all_components, open_component_group from ess.reduce.nexus.types import NeXusAllLocationSpec, NeXusLocationSpec from ess.spectroscopy.types import ( @@ -59,20 +60,85 @@ def load_analyzers(file_spec: NeXusFileSpec[RunType]) -> Analyzers[RunType]: ) -def _get_analyzer_for_detector_name( - detector_name: str, analyzers: Analyzers[RunType] -) -> Analyzers[RunType]: - detector_index = int(detector_name.split('_', 1)[0]) - analyzer_index = str(detector_index - 2) - for name, analyzer in analyzers.items(): - if name.startswith(analyzer_index): - return analyzer - raise RuntimeError(f"No analyzer found for detector {detector_name}") +def _do_breadth_first_search(group, targets, obj_deque, obj_next): + """ + Look for a unique element of targets by following the 'next' for object in a queue + + Parameters + ---------- + group: HDF5 Group + The group that contains all possible next named groups + targets: + A structure with named targets that supports `name in targets` + obj_deque: + A queue.deque of HDF5 Groups to be checked + obj_next: + A function that extracts a list of named groups to check from a given group + """ + while len(obj_deque) > 0: + check = obj_next(obj_deque.popleft()) + matches = [element for element in check if element in targets] + if len(matches) > 1: + raise ValueError("Non-unique elmeent match") + if len(matches) == 1: + return matches[0] + for element in check: + obj_deque.append(group[element]) + raise ValueError("No unique element found") + + +def analyzer_search(hdf5_instrument_group, analyzers, hdf5_detector_group): + """ + Use a NeXus Group's @inputs attribute to find an analyzer given a detector group + + Parameters + ---------- + hdf5_instrument_group: hdf5.Group + works if inside of a context group + ``` + scippnexus.File(filename, 'r') as f: + hdf5_instrument_group = f['/entry/instrument'] + ``` + analyzers: Anything with __contains__(str), e.g. dict[str, hdf5.Group] + Something to identify whether we've found a valid analyzer (by name) + hdf5_detector_group: hdf5.Group + any of f['/entry/detector'][scippnexus.NXdectector].values() + """ + from queue import deque + + from h5py import Group + + def obj_inputs(obj: Group) -> list[str]: + """Return the specified preceding component(s) list""" + if 'inputs' not in obj.attrs: + raise ValueError('@inputs attribute required for this search to work') + val = obj.attrs['inputs'] + # Deal with nexusformat (Python module) or kafka-to-nexus (filewriter) + # silently converting a len(list[str]) == 1 attribute to a str attribute: + return [val] if isinstance(val, str) else val + + return _do_breadth_first_search( + hdf5_instrument_group, analyzers, deque([hdf5_detector_group]), obj_inputs + ) + + +def get_detector_analyzer_map(file_spec: NeXusFileSpec[RunType]) -> DetectorAnalyzerMap: + """Probably not the right sciline way to do this.""" + + from scippnexus import File, NXcrystal, NXdetector + + filename = file_spec.value + with File(filename, 'r') as file: + inst = file['entry/instrument'] + analyzers = inst[NXcrystal] + detectors = inst[NXdetector] + return {k: analyzer_search(inst, analyzers, v) for k, v in detectors.items()} def analyzer_for_detector( analyzers: Analyzers[RunType], detector_location: NeXusComponentLocationSpec[snx.NXdetector, RunType], + detector_analyzer_map: DetectorAnalyzerMap, ) -> Analyzer[RunType]: """Extract the analyzer for a given detector. @@ -98,8 +164,14 @@ def analyzer_for_detector( """ if detector_location.component_name is None: raise ValueError("Detector component name is None") + if ( + analyzer_name := detector_analyzer_map.get(detector_location.component_name) + ) is None: + raise RuntimeError( + f"No analyzer found for detector {detector_location.component_name}" + ) analyzer = snx.compute_positions( - _get_analyzer_for_detector_name(detector_location.component_name, analyzers), + analyzers[analyzer_name], store_transform='transform', ) return Analyzer[RunType]( @@ -117,4 +189,5 @@ def analyzer_for_detector( load_instrument_angle, load_sample_angle, moderator_class_for_source, + get_detector_analyzer_map, ) diff --git a/src/ess/bifrost/types.py b/src/ess/bifrost/types.py index 46a0018..38647c2 100644 --- a/src/ess/bifrost/types.py +++ b/src/ess/bifrost/types.py @@ -21,3 +21,6 @@ class McStasRawDetector(sciline.Scope[RunType, sc.DataArray], sc.DataArray): ... ArcEnergy = NewType('ArcEnergy', sc.Variable) + + +DetectorAnalyzerMap = NewType('DetectorAnalyzerMap', dict[str, str])