Repository navigation
[essreduce] On-the-fly wavelength lookup table in generic workflow - #603
Conversation
Build the LUT pipeline via GenericUnwrapWorkflow(wavelength_from='analytical') instead of the simulation-based LookupTableWorkflow, following the essreduce rewrite that moved the wavelength lookup table into the generic workflow. Analytical mode derives the table from chopper-cascade polygon geometry: far faster than the tof neutron simulation and needs no simulated-neutron count. - SourcePosition -> Position[NXsource, AnyRun]; LtotalRange / PulseStride / LookupTable re-parametrised with [AnyRun, NXdetector]. - Drop NumberOfSimulatedNeutrons and the Simulation params model. - Synthetic chopper test fixture uses negative (anti-clockwise) chopper frequencies; a positive frequency drives essreduce's analytical chopper cascade into a degenerate frame (reported on scipp/ess#603). Depends on an unreleased ess.reduce: the analytical wavelength-LUT API lives on the essreduce lut-in-generic-wf branch and is absent from the latest release (26.5.0). The essreduce dependency pin must be bumped once that work is released before this can merge. Co-Authored-By: Claude Opus 4.8 <[email protected]>
…t origins of source bounds magic numbers
|
I added if len(b) < 2:
continueinside the loop. Does that fix it for you? |
| LookupTable, | ||
| LookupTableFilename, | ||
| Lut, | ||
| # Lut, |
| if len(b) < 2: | ||
| # This can happen if the polygon is degenerate (collapsed to a single | ||
| # vertex). | ||
| continue |
There was a problem hiding this comment.
Are you implying that the code below that also uses b and the bounds works nevertheless?
There was a problem hiding this comment.
I think it does?
But maybe you think it's safer to just discard the polygon if it has collapsed?
Can you clarify what a collapsed polygon means? Should there just be no polygon at all, or is there a single infinitessimally small point where neutrons could get through? (maybe it amounts to the same anyway)
There was a problem hiding this comment.
Good question. Does scippneutron keep around such degenerate polygons? Should they be dropped?
There was a problem hiding this comment.
From private conversation:
So I looked at the subframes in your exampl (Minimal reproducer).
the subframe that is still there is not just a single vertex, it's made up of 3 vertices and they are all the same: [0, 0, 0] in both time and wavelength.
It's a peculiarity that happens because we allow a 0 wavelength (the default bounds of the pulse) which is infinite speed.
The choppers are open just before t=0 and close exactly at t=0.
I'm thinking that the neutrons with infinite speed can make it through just at t=0, but neutrons with infinite speed that started at t=1ms don't make it through because they were born too late.
So we are left with a single point at the corner of the pulse that can make it through.
If I change the pulse bounds to start at 0.001 angstrom the error goes away.
I get a new error which crashes when the subframes are empty, but I also fixed that now.
Build the LUT pipeline via GenericUnwrapWorkflow(wavelength_from='analytical') instead of the simulation-based LookupTableWorkflow, following the essreduce rewrite that moved the wavelength lookup table into the generic workflow. Analytical mode derives the table from chopper-cascade polygon geometry: far faster than the tof neutron simulation and needs no simulated-neutron count. - SourcePosition -> Position[NXsource, AnyRun]; LtotalRange / PulseStride / LookupTable re-parametrised with [AnyRun, NXdetector]. - Drop NumberOfSimulatedNeutrons and the Simulation params model. - Synthetic chopper test fixture uses negative (anti-clockwise) chopper frequencies; a positive frequency drives essreduce's analytical chopper cascade into a degenerate frame (reported on scipp/ess#603). Depends on an unreleased ess.reduce: the analytical wavelength-LUT API lives on the essreduce lut-in-generic-wf branch and is absent from the latest release (26.5.0). The essreduce dependency pin must be bumped once that work is released before this can merge. Co-Authored-By: Claude Opus 4.8 <[email protected]>
Build the LUT pipeline via GenericUnwrapWorkflow(wavelength_from='analytical') instead of the simulation-based LookupTableWorkflow, following the essreduce rewrite that moved the wavelength lookup table into the generic workflow. Analytical mode derives the table from chopper-cascade polygon geometry: far faster than the tof neutron simulation and needs no simulated-neutron count. - SourcePosition -> Position[NXsource, AnyRun]; LtotalRange / PulseStride / LookupTable re-parametrised with [AnyRun, NXdetector]. - Drop NumberOfSimulatedNeutrons and the Simulation params model. - Synthetic chopper test fixture uses negative (anti-clockwise) chopper frequencies; a positive frequency drives essreduce's analytical chopper cascade into a degenerate frame (reported on scipp/ess#603). Depends on an unreleased ess.reduce: the analytical wavelength-LUT API lives on the essreduce lut-in-generic-wf branch and is absent from the latest release (26.5.0). The essreduce dependency pin must be bumped once that work is released before this can merge. Co-Authored-By: Claude Opus 4.8 <[email protected]>
Insert the providers from the LUT workflow into the GenericUnwrapWorkflow to compute the lut on the fly with chopper cascade.
Breaking change: we compute a LUT per component (detector bank, monitor) instead of one table for all.
Building small tables is faster, and interpolating inside a small table is also faster (most probably due to cache misses in larger tables).
By default, the workflow tries to load the lookup table from file, which behaves like the old code.
The workflow now has 3 modes it can operate in:
wavelength_from='file': the default = read pre-computed table from filewavelength_from='simulation': on-the-fly computed table using tof simulation from chopper params (slow)wavelength_from='analytical': on-the-fly computed table using chopper_cascade -> what we want to default to in the future.Suggestions for a better name than
'analytical'are welcome.The idea is that each package would then move over to
analyticalmode when they wish to do so (in follow-up PRs).Additional notes:
I tried to reduce the number of parameters we need to set on the workflow:
LtotalRangeis now determined from the pixel positions of the detector bank, or the monitor position.PulseStrideis now guessed from the chopper frequencies (see comment here)Finally, the
LookupTableWorkflowthat was used to create lookup tables usingtofis now just an alias for theGenericUnwrapWorkflowthat usestofby default, so that the behaviour remains the same as before. We plan to remove the alias in the future.