Skip to content

[essimaging] Use on-the-fly wavelength lookup table for Odin#631

Merged
nvaytet merged 13 commits into
mainfrom
imaging-onthefly-lut
Jun 11, 2026
Merged

[essimaging] Use on-the-fly wavelength lookup table for Odin#631
nvaytet merged 13 commits into
mainfrom
imaging-onthefly-lut

Conversation

@nvaytet

@nvaytet nvaytet commented Jun 9, 2026

Copy link
Copy Markdown
Member

Move the Odin Bragg edge workflow to build the wavelength lookup table on-the-fly from chopper parameters in the file.

For now, we patch the chopper settings on the workflow, as the info is incomplete in the template coda file that was used to create the test files.

Needs scipp/scippneutron#707

@nvaytet nvaytet added the essimaging Issues for essimaging. label Jun 9, 2026
@github-actions github-actions Bot added the essreduce Issues for essreduce. label Jun 9, 2026
disk_choppers = {
disk_choppers = {}
for key, ch in choppers.items():
if (

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.

If a chopper has some empty logs, it is dropped as it is considered parked/not in use.

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.

Do you know how this will be handled in production? Will nexus files omit parked choppers or will they always be there? We shouldn't emit a warning if length-0 logs are a normal thing.

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.

My assumption is that the choppers will always be there, because they are in the template files used by ecdc.
But I asked now in the ecdc channel.

We shouldn't emit a warning if length-0 logs are a normal thing.

True. Let's see what ecdc say and then we can probably remove the warning.

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.

No reply from ECDC so far.
I removed the warning for now, as I am quite sure parked choppers will be in the files.

# Run 'tox -e deps' after making changes here. This will update requirement files.
# Make sure to list one dependency per line.
dependencies = [
"dask>=2022.1.0",

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.

All these dependencies are brought in by essreduce.
Removing them here avoids the issue of always keeping all the lower bounds up to date (right now, most of the lower bounds we had were lying anyway).

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.

I would still list all direct dependencies here. Or at least direct 3rd party dependencies (dask, graphviz) to make sure we don't rely on essreduce's setup. We don't need to list lower bounds here as long as they are not tighter than essreduce's bounds.

@nvaytet nvaytet requested a review from jl-wynen June 10, 2026 14:46
@nvaytet nvaytet added this pull request to the merge queue Jun 11, 2026
Merged via the queue into main with commit 4d12e02 Jun 11, 2026
24 checks passed
@nvaytet nvaytet deleted the imaging-onthefly-lut branch June 11, 2026 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

essimaging Issues for essimaging. essreduce Issues for essreduce.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants