Skip to content

[ESSSPECTROSCOPY] Update BIFROST NeXus loader for angles - #671

Merged
jl-wynen merged 4 commits into
mainfrom
bifrost-load-nexus-parameters
Jul 23, 2026
Merged

[ESSSPECTROSCOPY] Update BIFROST NeXus loader for angles#671
jl-wynen merged 4 commits into
mainfrom
bifrost-load-nexus-parameters

Conversation

@jl-wynen

Copy link
Copy Markdown
Member

The integration tests are failing because of this. It looks like the CODA files encode the angles we need.

I am unhappy with relying on group names this much but I don't see a better alternative given that there are no unique NeXus classes to look for.

@jl-wynen

Copy link
Copy Markdown
Member Author

Should I add test data for this? It would be a fairly large file with bogus data. So it wouldn't be useful beyond testing this loader.

@github-actions github-actions Bot added the essspectroscopy Issues for essspectroscopy. label Jul 23, 2026
@github-actions github-actions Bot changed the title Update BIFROST NeXus loader for angles [ESSSPECTROSCOPY] Update BIFROST NeXus loader for angles Jul 23, 2026
Comment on lines +42 to +47
with open_component_group(
NeXusLocationSpec(filename=file_spec.value),
nx_class=snx.NXparameters,
parent_class=snx.NXentry,
) as group:
return group[param_name][()]['value']

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So McStas stores this differently than PROD?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The McStas files that Greg made have an NXparameters group which doesn't exist in prod because it holds simulation params. The handling of nested value groups in the prod loader can load those data from mcstas files. Unfortunately, the sample stack does not exist in mcstas files. But the instrument angle can be loaded. So I could remove its special handler for mcstas. Should I?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe?

Comment thread packages/essspectroscopy/src/ess/bifrost/io/nexus.py Outdated
Comment on lines +59 to +62
if isinstance(log['value'], snx.Group):
# The NXlog is nested in this group, e.g., the group can be an NXpositioner
# with children 'value', 'target_value', etc.
transform: snx.nxtransformations.Transform = log['value'][()] # type: ignore[assignment]

@SimonHeybrock SimonHeybrock Jul 23, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still trying to understand whether this kind of nesting is a bug/bad and should be reported. Have had issues with this elsewhere. But no action here, as I presume it is necessary to make it work now.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it wouldn't work otherwise.

@jl-wynen
jl-wynen force-pushed the bifrost-load-nexus-parameters branch from b690076 to db2d8bf Compare July 23, 2026 11:07
@jl-wynen
jl-wynen added this pull request to the merge queue Jul 23, 2026
Merged via the queue into main with commit 9fa8d3f Jul 23, 2026
6 checks passed
@jl-wynen
jl-wynen deleted the bifrost-load-nexus-parameters branch July 23, 2026 12:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

essspectroscopy Issues for essspectroscopy.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants