Skip to content
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
wants to merge 19 commits into
base: main
Choose a base branch
from

Conversation

GuiMacielPereira
Copy link
Collaborator

@GuiMacielPereira GuiMacielPereira commented Sep 6, 2024

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 .

@GuiMacielPereira GuiMacielPereira linked an issue Sep 6, 2024 that may be closed by this pull request
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.
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Convert AnalysisReduction object into Mantid algorithm
2 participants