Skip to content

Distinguish between raw and parsed choppers #242

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
jl-wynen merged 1 commit into from
May 19, 2025
Merged

Distinguish between raw and parsed choppers #242

jl-wynen merged 1 commit into from
May 19, 2025

Conversation

@jl-wynen
Copy link
Member

@jl-wynen jl-wynen commented May 14, 2025 •
edited
Loading

The tof workflow incorrectly assumes that loaded Choppers are DiskChopper objects. In reality, they are DataGroups. This means that the tof workflow fails at during compute. This PR splits the Choppers type into two. One for the nested data groups loaded from NeXus and one for processed DiskChopper objects. There is no provider to connect the two because that is a complicated process that involves finding plateaus and extracting scalar quantities from the NeXus data. See https://scipp.github.io/scippneutron/user-guide/chopper/processing-nexus-choppers.html

If the data is completely flat, this can be simple. E.g., for the simulated BIFROST data, we can simply do this:

def extract_chopper_plateau(chopper):
    processed = chopper.copy()
    # These are constant in the simulated data.
    processed['rotation_speed'] = processed['rotation_speed'].data.mean()
    processed['phase'] = processed['phase'].data.mean()
    # Guessing here as this is not stored in the file.
    processed['beam_position'] = sc.scalar(0.0, unit='deg')
    return DiskChopper.from_nexus(processed)


def extract_chopper_plateaus(choppers: RawChoppers[RunType]) -> DiskChoppers[RunType]:
    return DiskChoppers[RunType](choppers.apply(extract_chopper_plateau))

workflow.insert(parse_disk_choppers)

@github-project-automation github-project-automation bot moved this to In progress in Development Board May 14, 2025
@jl-wynen jl-wynen moved this from In progress to Selected in Development Board May 14, 2025
@jl-wynen jl-wynen marked this pull request as ready for review May 14, 2025 14:34
@jl-wynen jl-wynen mentioned this pull request May 14, 2025
Merged
@SimonHeybrock
Copy link
Member

SimonHeybrock commented May 16, 2025

The tof workflow incorrectly assumes that loaded Choppers are DiskChopper objects.

So how did the workflow work so far? Have chopper params always been set by hand (using the "incorrect" type)?

@jl-wynen
Copy link
Member Author

jl-wynen commented May 16, 2025

The tof workflow incorrectly assumes that loaded Choppers are DiskChopper objects.

So how did the workflow work so far? Have chopper params always been set by hand (using the "incorrect" type)?

I think so. I don't see any other way that would have worked. In particular for DREAM, we never (to my knowledge) used nexus files, only a hard coded list of choppers.

@SimonHeybrock SimonHeybrock requested a review from nvaytet May 16, 2025 11:17
@nvaytet
Copy link
Member

nvaytet commented May 19, 2025

If the data is completely flat, this can be simple. E.g., for the simulated BIFROST data, we can simply do this:

Note that ECDC has started putting the 'setpoint' values for the rotation speed and phase in the nexus files, which is the target value that the control software is supposed to achieve. So in principle, if we trust that, we could just read that instead of finding plateaus (which would remain useful for debugging).
Screenshot_20250422_181511

nvaytet
nvaytet approved these changes May 19, 2025
View reviewed changes
src/ess/reduce/nexus/workflow.py
) -> Choppers[RunType]:
"""Convert the NeXus representation of a chopper to ours."""
return Choppers[RunType](
) -> RawChoppers[RunType]:
Copy link
Member

@nvaytet nvaytet May 19, 2025

Choose a reason for hiding this comment

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

The tof workflow incorrectly assumes that loaded Choppers are DiskChopper objects.

So how did the workflow work so far? Have chopper params always been set by hand (using the "incorrect" type)?

I think so. I don't see any other way that would have worked. In particular for DREAM, we never (to my knowledge) used nexus files, only a hard coded list of choppers.

I think this is correct, we read and used choppers from nexus for the TBL but in that file, the list of choppers was empty, so we then just built a LUT without choppers.

@jl-wynen
Copy link
Member Author

jl-wynen commented May 19, 2025

If the data is completely flat, this can be simple. E.g., for the simulated BIFROST data, we can simply do this:

Note that ECDC has started putting the 'setpoint' values for the rotation speed and phase in the nexus files, which is the target value that the control software is supposed to achieve. So in principle, if we trust that, we could just read that instead of finding plateaus (which would remain useful for debugging). Screenshot_20250422_181511

Good to know! If these setpoints are good enough, we can have a default provider that maps RawChoppers to DiskChoppers.

@jl-wynen jl-wynen enabled auto-merge May 19, 2025 08:41
@SimonHeybrock
Copy link
Member

SimonHeybrock commented May 19, 2025

If the data is completely flat, this can be simple. E.g., for the simulated BIFROST data, we can simply do this:

Note that ECDC has started putting the 'setpoint' values for the rotation speed and phase in the nexus files, which is the target value that the control software is supposed to achieve. So in principle, if we trust that, we could just read that instead of finding plateaus (which would remain useful for debugging). Screenshot_20250422_181511

Good to know! If these setpoints are good enough, we can have a default provider that maps RawChoppers to DiskChoppers.

We might also want to avoid loading the complete timestamp logs, since they might get quite big?

@jl-wynen jl-wynen merged commit 99933a7 into main May 19, 2025
4 checks passed
@jl-wynen jl-wynen deleted the parse-choppers branch May 19, 2025 08:46
@github-project-automation github-project-automation bot moved this from Selected to Done in Development Board May 19, 2025
@jl-wynen jl-wynen mentioned this pull request Jul 1, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nvaytet nvaytet nvaytet approved these changes

Assignees

No one assigned

Labels

None yet

Projects

Development Board
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jl-wynen @SimonHeybrock @nvaytet