-
Notifications
You must be signed in to change notification settings - Fork 3
Fix run mapping functionality #212
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
| for part in (Numerator, Denominator): | ||
| pipeline[CleanSummedQ[key, part]] = ( | ||
| pipeline[CleanSummedQ[key, part]] | ||
| pipeline[WavelengthScaledQ[key, part]] = ( |
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 guess my only question is: did the results change at all after this modification?
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.
Before this fix it either did not work, or the monitor term from a single run was used for all, i..e, it should be different.
Co-authored-by: Jan-Lukas Wynen <j-l.wynen@hotmail.de>
Co-authored-by: Jan-Lukas Wynen <j-l.wynen@hotmail.de>
Co-authored-by: Jan-Lukas Wynen <j-l.wynen@hotmail.de>
Support ISIS histogram files
| Filename=str(filename), LoadMonitors=True, StoreInADS=False | ||
| ) | ||
| # Loading many small files with Mantid is, for some reason, very slow when using | ||
| # the default number of threads in the Dask threaded scheduler (1 thread worked |
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.
Can it be because multi-threading is also used internally by Mantid when it is loading the data?
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 think I checked when I first wrote this (note this code and comment was simply moved in this PR). I don't think so.
Fixes #211.
Note
This is not addressed in this PR. Is this something that needs fixing?