Fix chopper rotations in optimize_for and Chopper#144
Merged
Conversation
Comment on lines
+226
to
+239
| nrot = ( | ||
| max( | ||
| int( | ||
| sc.ceil( | ||
| ( | ||
| self.omega * time_limit.to(unit='s') | ||
| - smallest_angle.to(unit='rad') | ||
| ) | ||
| / two_pi | ||
| ).value | ||
| ), | ||
| 1, | ||
| ) | ||
| + 1 |
Member
There was a problem hiding this comment.
Can a helper variable help with readability here? This is too many lines :D
| sc.ceil( | ||
| ( | ||
| self.omega * time_limit.to(unit='s') | ||
| - smallest_angle.to(unit='rad') |
Member
There was a problem hiding this comment.
It is not completely obvious whether this is correct for "very positive" openings. there are tests like test_chopper_with_very_negative_angles_performs_enough_rotations_to_cover_timescale, but what happens if the smallest angle is so large that one would need to rotate backward to cover the full range?
Member
Author
There was a problem hiding this comment.
Good point, we probably need a safety net in the other direction as well!
SimonHeybrock
approved these changes
Jun 24, 2026
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.
When trying to use
optimize_forin the Trex instrument case, no neutrons were making it through.The pulse-shaping and monochromatic choppers were not performing enough rotations and were thus blocking all neutrons.
Giving them a timescale to dictate how long they should rotate for helped (in this PR also), but did not fix everything.
The pulse-shaping chopper 2 was still not performing enough rotations.
It was because it has some cutout angles < -360 deg, meaning that it was still missing at least one rotation to cover the requested timescale.
Instead of guessing from the rotation frequency and timescale alone, we now find the smallest cutout angle and compute how many rotations need to be done to reach the requested timescale.
We can now make a T-rex model, optimized for the choppers in the beamline, with 0 blocked neutrons:


@bingli621