-
Notifications
You must be signed in to change notification settings - Fork 1
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
Analysis reduction to Mantid Algorithm #132
Open
GuiMacielPereira
wants to merge
19
commits into
main
Choose a base branch
from
analysis-reduction-to-mantid-algo
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
2 tasks
Set main properties for the input arguments
This change is to starting point to pass the profile inputs as a table workspace
Previously the profiles were accessed through a list of bespoke dataclass objects called NeutronComptonProfile. Made the change to use a table workspace instead, to conform with the new input argument. Still need to change the way the Joint routine links profiles together.
Cleaned some unused comments from previous changes to the code. Made tests use routine object directly without having to go through the AlgorithmManager, this makes sure we can access results from a public method.
Initialize fit parameters and bounds when setting up the class. This makes it easier to keep track of the order in which the masses are getting into the parameters array and makes it easier to change it in the future. Made self._masses method because it is used several times across the routine.
System tests are sliglty off so I increase the tolerance to have them pass.
GuiMacielPereira
force-pushed
the
analysis-reduction-to-mantid-algo
branch
from
September 10, 2024 15:17
5518599
to
b0991a0
Compare
Added unit tests and removed old comments
Transformed existing function to create a function that takes in an outputs table containing the means at the end of the routine and passes these to the inputs profiles table as the initial parameters. The lightest mass is always unfixed (bounds are relaxed). The h ratio is used depending on whether the sample contains hydrogen.
Refactored joint algorithm to fit new approach of using profiles table as inputs to the AnalysisReduction algorithm.
For consistency sake, I changed all of the suffixes of the workspaces to lower case and abreviated longer terms where possible. This makes the workspaces easier to read and looks a bit tidier.
When h ratio is not provided and hydrogen is present in the sample, the routine for estimating the h ratio to the lowest mass is triggered. Had to update this routine with the changes from converting vesuvio analysis into a mantid algorithm.
Used logging instead of print, also formatted the logging statements better.
Previously the constraints argument was disabled because I did not find a way to pass Python objects (tuple of dictionaries) into the mantid algorithm. This workaround converts the tuple of dictionaries into a byte string using the dill library which can then be converted to a normal string and passed into the mantid algorithm as an argument. When the algorithm executes, the string is converted into byte string again though eval() and then into the tuple of dicts of lambdas by the dill library. A better solution might be implemented in the future but this will suffice for now.
The fittnig routine was failing due to the naming conventions having changed. Updated routine so that it finds the correct ncp workspaces. Updated the system tests.
Cleaned up some comments, and introduced some others for clarity.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of work:
Turn analysis reduction algorithm into a Mantid algorithm.
Required changing several files since Mantid algorithms are limited to the public methods defined by
setProperty()
.This means that whilst before the ncp profiles could be passed in as a list of Python objects, in Mantid algorithms they need to be passed in as a
TableWorkspace
.I had to change the calculation of H ratios to work from this
TableWorkspace
.I also introduced a new library called
dill
which has the only purpose of passing Scipy constraints (a tuple of dictionaries containing lambdas) into a byte string so it can be passed to the Mantid algorithm as a string.Added a few unit tests for the new functions I introduced.
To test:
Code review and check that analysis system and unit tests passed.
Fixes #129 .