Repository navigation
fix: keep analyzer crystals and inputs in geometry artifacts - #1428
Merged
Merged
Conversation
BIFROST finds each detector's analyzer by following `inputs` from the detector to an NXcrystal, then uses the crystal's `d_spacing` and `depends_on` placement. The geometry artifact dropped the crystal fields (the group only existed as parent of its transformations) and the `inputs` dataset used by 2026-10 run files, so the lookup failed on the artifact. NXcrystal is now a copied component class (placement and `d_spacing`), and every group the artifact creates keeps its `inputs` dataset, as it already kept the older `inputs` attribute. Part of #1427 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Member
Author
|
LGTM |
1 of 17 tasks
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
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.
Part of #1427.
BIFROST finds each detector's analyzer by following
inputsfrom the detector to anNXcrystal, then takes the crystal'sd_spacingand computes its position fromdepends_on. The geometry artifact broke this in two ways. First,NXcrystalgroups existed only as empty parents of theirtransformations, withoutd_spacingordepends_on. Second, theinputsdataset used by 2026-10 run files was dropped. On the 2026-10 artifact the lookup therefore fails with "No NXcrystal found in the inputs of channel_1_1_triplet". On artifacts from older files (inputsstored as an attribute, which the script already kept) the crystal is found, but it cannot be placed.The rules are generic rather than BIFROST-specific:
NXcrystalis a copied component class. It keepsdepends_onandd_spacing. Its transformations are already copied like those of every other component.inputsdataset, just as it already keeps its attributes. Intermediate groups on the chain from detector to crystal (e.g.channel_*_detector_angle) already exist in the artifact because their transformations are copied, so they need no separate handling.inputson groups that point at components the artifact does not keep (guides, collimators) are left dangling. This does no harm: the lookup stops at the first crystal.Artifact size for the 2026-10 reference run file grows from 2.15 MB to 2.21 MB.
The 2026-10 files store
inputsas a scalar dataset, which the released essspectroscopy lookup cannot read. Using this with those files needs scipp/ess#827. On its own, the change fixes artifacts from files with the older attribute encoding: before it, crystal placement failed there with any essspectroscopy version.Test plan
inputschain (dataset and attribute encoding, through an intermediate group) and the crystal fields. One test runsload_analyzer_for_detectoron the artifact.coda_bifrost_999999_00026862.hdf) and the older McStas file. On the 2026-10 artifact, with the [ESSSPECTROSCOPY] fix: BIFROST loaders for 2026-10 run files ess#827 lookup, all 45 detectors resolve to distinct crystals (channel_1_1_triplet→channel_1_1_monochromator,d_spacing3.355 Å), and the crystal position computes, as an empty time series because the tank angle is an NXlog the artifact trims. On the McStas artifact all 45 detectors resolve and place with both the released and the fixed lookup. Before the change, both artifacts failed.🤖 Generated with Claude Code