Support multiple pulses in chopper cascade#700
Conversation
| subframes = [ | ||
| Subframe(time=time + i * pulse_period, wavelength=wavelength) | ||
| for i in range(npulses) | ||
| ] |
There was a problem hiding this comment.
Should there be source pulses also before the "pulse of interest? Or is that handled by some wrapping you do afterwards?
There was a problem hiding this comment.
I do some wrapping manually afterwards in essreduce. I thought I would keep it simple here, so that it behaves a little like tof. But I don't mind either way.
There was a problem hiding this comment.
Its fine as it is, probably. But I do wonder if one could formulate this on a cyclic coord system instead?
There was a problem hiding this comment.
Maybe something like making the period boundary act like a chopper and chop the polygons as well?
There was a problem hiding this comment.
What I mean: After every time propagation, do a mod 71 ms or (142 ms if pulse-skipping, etc.). But the polygon math might not be easy to port.
There was a problem hiding this comment.
Yes I got that. You'd have to do the mod, but as you say you would run into cases where some polygons span across the period boundary.
I was thinking one easy way to split those polygons would be to use the chop mechanism?
You would then of course have to duplicate the pulses to the frame before the current frame.
There was a problem hiding this comment.
I don't think pulse duplication would be necessary any more, since periodicity takes care of that? Splitting the polygons that cross the boundary is a good suggestion, provided it does not interfere with other code that tries to look at subframes?
| for i in range(3): | ||
| expected_subframe = chopper_cascade.Subframe( | ||
| time=expected_time + i * pulse_period.to(unit='ms'), | ||
| wavelength=expected_wavelength, | ||
| ) |
There was a problem hiding this comment.
Not a fan of repeating the implementation-math in the test, but not sure what a better test would be.
There was a problem hiding this comment.
I could check that the min of frame.bounds().time is zero, and the max is 2 * period + 1ms?
And remove the looping over the frames. Not sure it's better, but it's different.
Example: