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
Add new integration algorithm for single-crystal Bragg peaks #37103
Conversation
Algorithm is adapted and extended from an algorithm in Gutmann, M. J. (2005). SXD2001. ISIS Facility, RAL
Unfortunately Nelder-Mead minimizer in scipy does not support constraints, otheriwse it would be good to pass this to minimize as a NonlinearConstraint
System test previously not run due to indentation level typo. When enabled the test failed due to a bug trying to get the shoebox dimensions from a strong peak when there was no output file specified (as no ShoboxResult associated with the peak).
Not sure why the discrepancy with my local build - recent change to numpy version?
8a1046c
to
2888cc4
Compare
Help see why the test is failing!
2888cc4
to
af0df07
Compare
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.
Looks good, still need to test. Spotted a few typos but other than that the code is neatly divided into functions where possible :)
Framework/PythonInterface/mantid/api/src/Exports/IPeakFunction.cpp
Outdated
Show resolved
Hide resolved
Framework/PythonInterface/plugins/algorithms/IntegratePeaksShoeboxTOF.py
Outdated
Show resolved
Hide resolved
Framework/PythonInterface/plugins/algorithms/IntegratePeaks1DProfile.py
Outdated
Show resolved
Hide resolved
Framework/PythonInterface/plugins/algorithms/IntegratePeaks1DProfile.py
Outdated
Show resolved
Hide resolved
Framework/PythonInterface/plugins/algorithms/IntegratePeaks1DProfile.py
Outdated
Show resolved
Hide resolved
Framework/PythonInterface/plugins/algorithms/IntegratePeaks1DProfile.py
Outdated
Show resolved
Hide resolved
Framework/PythonInterface/plugins/algorithms/IntegratePeaks1DProfile.py
Outdated
Show resolved
Hide resolved
Framework/PythonInterface/plugins/algorithms/IntegratePeaks1DProfile.py
Outdated
Show resolved
Hide resolved
Framework/PythonInterface/plugins/algorithms/IntegratePeaks1DProfile.py
Outdated
Show resolved
Hide resolved
👋 Hi, @RichardWaiteSTFC, Conflicts have been detected against the base branch. Please rebase your branch against the base branch. |
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.
Description of work
Add new integration algorithm for single-crystal Bragg peaks adapted and extended from an algorithm in SXD2001 [1].
It uses scipy.minimize to fit peaks in adjacent pixels until the I/sigma ratio drops below some threshold - note scipy.minimize is used because mantid fitting currently does not perform well for this type of function - in principle it should be straightforward to use the mantid fitting at a later date.
[1] Gutmann, M. J. (2005). SXD2001. ISIS Facility, Rutherford Appleton Laboratory, Oxfordshire, England.
Fixes #33629
Report to: Matthias Gutmann (SXD)
To test:
(1) Run this script (maybe delete a few rows/peaks from the table if it takes too long)
(2) Check the output pdf (it should have a couple of plots per peak)
Reviewer
Please comment on the points listed below (full description).
Your comments will be used as part of the gatekeeper process, so please comment clearly on what you have checked during your review. If changes are made to the PR during the review process then your final comment will be the most important for gatekeepers. In this comment you should make it clear why any earlier review is still valid, or confirm that all requested changes have been addressed.
Code Review
Functional Tests
Does everything look good? Mark the review as Approve. A member of
@mantidproject/gatekeepers
will take care of it.Gatekeeper
If you need to request changes to a PR then please add a comment and set the review status to "Request changes". This will stop the PR from showing up in the list for other gatekeepers.