-
Notifications
You must be signed in to change notification settings - Fork 2
Use essreduce nexus workflow #84
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.
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
Conversation
tests/dream/geant4_reduction_test.py
Outdated
| assert sc.identical(result.coords['dspacing'], params[DspacingBins]) | ||
| assert sc.allclose(result.coords['two_theta'], params[TwoThetaBins]) | ||
| assert sc.identical(result.coords['dspacing'], workflow.compute(DspacingBins)) | ||
| assert sc.allclose(result.coords['two_theta'], workflow.compute(TwoThetaBins)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer the old implementation because if checks that the workflow respects the parameters. The new tests only check that the workflow's outputs are consistent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refactored, see update.
Co-authored-by: Jan-Lukas Wynen <jan-lukas.wynen@ess.eu>
The recent additions in #78 have mostly been moved into ESSreduce. We can thus remove them here, and forward to ESSreduce.
Also renamed
EmptyCanRun->BackgroundRunandEmptyInstrumentRun->EmptyBeamRun. While we do not have actual scientist feedback on this I think using common names is important. I expect some future back and forth on the actual names (e.g., isBackgroundRuna good name?), but I hope that can be a global discussion across techniques, i.e., we would rename the types in ESSreduce in that case.The Geant4 loader sees some small changes, partially relying on the providers from the NeXus workflow, but overriding some details.
Depends on an releases of scipp/essreduce#81,
CI here will fail for now.