Add absorbtion corrections to the OSIRIS scripts #35430

MohamedAlmaki · 2023-04-03T15:42:52Z

Description of work.
This PR implements absorption corrections in the OSIRIS scripts, which can be applied in two ways: Simple Absorption Correction and Paalman Pings Absorption Correction.

For Simple Absorption Correction, the MonteCarloAbsorptionCorrection algorithm is used to correct the workspaces. For Paalman Pings Absorption Correction, the PaalmanPingsMonteCarloAbsorption algorithm is used.

To enable absorption correction in the focus function, absorb_corrections parameter must be set to True. The user can select the absorption correction method by setting empty_can_subtraction_method to either Simple or PaalmanPings.

Here's an example of Simple Absorption Correction:

osiris_focus.create_vanadium(run_number="120033,120043")

osiris_focus.focus(run_number="120033,120034",
    merge_drange=True,
    vanadium_normalisation=True,
    absorb_corrections=True,
    empty_can_subtraction_method="Simple",
    subtract_empty_can=True,
    sample_empty_scale=0.05,
)

And an example of Paalman Pings Absorption Correction:

osiris_focus.create_vanadium(run_number="120033,120043")

osiris_focus.focus(run_number="120033,120034",
    merge_drange=True,
    vanadium_normalisation=True,
    absorb_corrections=True,
    empty_can_subtraction_method="PaalmanPings",
    paalman_pings_events_per_point=1000
)

Since absorption correction can increase the intensity of peaks, it's possible to get very high values after applying vanadium normalization. To prevent this, vanadium_normalization_cutoff has been added. This parameter limits the maximum values for the counts after applying the vanadium normalization.

To test:
Here is the testing script:

from mantid.simpleapi import *
import matplotlib.pyplot as plt
import numpy as np
from isis_powder.osiris import Osiris
from isis_powder import SampleDetails

osiris_focus = Osiris(user_name="Calib",
                  output_directory=r"\osiris\OsirisCalibrationFiles\output",
                  config_file=r"\osiris\OsirisCalibrationFiles\Config.yaml",
                  calibration_directory=r"\osiris\OsirisCalibrationFiles\Calibration_files",
                  calibration_mapping_file=r"\osiris\OsirisCalibrationFiles\Calibration_files\osiris_run_mapping.yaml"
)

sample_details = SampleDetails(radius=1.1, height=8, center=[0, 0, 0], shape='cylinder')
sample_details.set_material(chemical_formula='Cr2-Ga-N', number_density=10.0)
sample_details.set_container(
    chemical_formula='Al',
    number_density=2.7,
    radius=1.2
)

osiris_focus.set_sample_details(sample=sample_details)

osiris_focus.create_vanadium(run_number="120033,120043")

osiris_focus.focus(run_number="120033,120043",
    merge_drange=True,
    vanadium_normalisation=True,
    absorb_corrections=True,
    empty_can_subtraction_method="PaalmanPings",
    vanadium_normalization_cutoff=1000000,
    paalman_pings_events_per_point=1000
)

Contact me for the configuration and calibration files.

Fixes #34311.

Reviewer

Please comment on the following (full description):

Code Review

Is the code of an acceptable quality?
Does the code conform to the coding standards?
Are the unit tests small and test the class in isolation?
If there is GUI work does it follow the GUI standards?
If there are changes in the release notes then do they describe the changes appropriately?
Are the release notes saved in a separate file, using Issue or PR number for file name and in the correct location?

Functional Tests

Do changes function as described? Add comments below that describe the tests performed?
Do the changes handle unexpected situations, e.g. bad input?
Has the relevant (user and developer) documentation been added/updated?

Does everything look good? Mark the review as Approve. A member of @mantidproject/gatekeepers will take care of it.

SpencerHowells · 2023-04-03T16:23:24Z

@MohamedAlmaki Thers's no need for the option of Simple or PP. PP will do both.

MohamedAlmaki · 2023-04-04T08:59:42Z

@MohamedAlmaki Thers's no need for the option of Simple or PP. PP will do both.

@SpencerHowells I added this option because other diffraction instruments have similar options in their scripts, and we wanted to create a standardized interface across all diffraction instruments. Does this option irrelevant to OSIRIS specifically and other instruments can benefit from it?

scripts/Diffraction/isis_powder/osiris.py

AnthonyLim23 · 2023-04-06T13:24:51Z

I have removed myself as I am going to be on holiday and dont want to block this getting reviewed

AnthonyLim23 · 2023-05-02T15:02:15Z

scripts/Diffraction/isis_powder/osiris.py

+        :param run_details: the run details of the workspace. Unused parameter added for API compatibility.
+        :return: A workspace containing the corrections.
+        """
+        events_per_point = 1000


should this be hard coded or an argument with a default?

It could be a parameter. I will add it as a parameter

AnthonyLim23 · 2023-05-02T15:05:36Z

Could you please add some tests for checking the new functionality.

AnthonyLim23 · 2023-06-16T09:16:43Z

Testing/SystemTests/tests/framework/ISIS_PowderOsirisTest.py

+            "Test_PaalmanPings_Absorb",
+            "OSI120032_d_spacing.nxs",
+            absorb_corrections=True,
+            empty_can_subtraction_method="PaalmanPings",


can we also have a test for the simple option too?

it is already added here

is there a reason they are not two tests in the same class?

I think from a separation of concerns perspective, it is better to implement every test in its own class. Also, it will be clearer to see every test separately in the test output

most tests in Manitd use one class with different options. Having a class for each one makes the file very long (repeated boiler plate) and makes it easy to miss things (e.g. me not spotting that you had the test)

Right, there is a lot of duplicate code. From my understanding, I guess using one class is the case in C++ but in Python, it is not. Because the test interface is designed to run one test you need to implement runTest and validate. So it will be strange to run two tests in runTest function. To reduce code duplication I can write a common test class for all the tests as in Pearl Tests. What do you think?

Ah ok, its a system test and not a unit test. Sorry I missed that, hence my comments.

A template class would help. I think thats a good idea.

No problem, ok, I will do that

AnthonyLim23 · 2023-06-20T10:58:06Z

The tests look much cleaner now. Thank you. Once the builds pass I will approve it

MohamedAlmaki · 2023-06-20T12:43:20Z

I didn't notice that there is a mismatch problem in one of the tests. I will fix it

github-actions · 2023-06-21T09:14:58Z

👋 Hi, @MohamedAlmaki,

Conflicts have been detected against the base branch. Please rebase your branch against the base branch.

AnthonyLim23

Looks good to me

forgot to test it

AnthonyLim23

it looks like it works

RichardWaiteSTFC

Just a couple of comments, I think it would be good to change one of the parameter names for consistency, but as for the ReplaceSpecialValues threshold, I think we can defer this to a separate PR

RichardWaiteSTFC · 2023-07-06T09:31:34Z

scripts/Diffraction/isis_powder/osiris_routines/osiris_param_mapping.py

@@ -25,5 +26,15 @@
    ParamMapEntry(ext_name="xye_filename", int_name="xye_filename"),
    ParamMapEntry(ext_name="sample_empty_scale", int_name="sample_empty_scale", optional=True),
    ParamMapEntry(ext_name="keep_raw_workspace", int_name="keep_raw_workspace", optional=True),
+    ParamMapEntry(ext_name="absorb_corrections", int_name="absorb_corrections"),


I think this should be named in a way that is consistent with the other instruments e.g.

mantid/scripts/Diffraction/isis_powder/polaris_routines/polaris_param_mapping.py

Line 20 in 5096764

ParamMapEntry(ext_name="do_absorb_corrections", int_name="do_absorb_corrections"),

I was going to do this but I noticed the previous parameters don't follow the convention of the other instruments (e.g. vanadium_normalization not do_vanadium_normalization). If I changed all of the parameters the users using the existing script will have to change it. In addition, the OSIRIS users probably don't use other instrument scripts so they will not notice this inconsistency. What do you think?

RichardWaiteSTFC · 2023-07-06T09:47:50Z

scripts/Diffraction/isis_powder/osiris.py

        :param run_details: The run details of the run number
        :return: the vanadium path
        """

        return run_details.unsplined_vanadium_file_path

+    def get_vanadium_normalization_cutoff(self):


Normally instrument settings are accessed directly rather than through getters (there are a handful of getters, so I accept it's a bit inconsistent). In this case I think you have done this so that you don't have to add the parameter for the other instruments (as this overrides an abstract_inst method that returns None).
However there are a few things that maybe are worth discussing?

Is this parameter ever likely to be changed (if not it can be hard-coded as it is in calibrate)?

mantid/scripts/Diffraction/isis_powder/routines/calibrate.py

Lines 50 to 56 in 5096764

aligned_ws = mantid.ReplaceSpecialValues(

InputWorkspace=aligned_ws,

OutputWorkspace=aligned_ws.name(),

NaNValue=0,

InfinityValue=0,

BigNumberThreshold=1e15,

SmallNumberThreshold=-1e15,

If we do want to add a parameter, as ReplaceSpecialValues is called in other places, is it worth having the same threshold for all instances?

And in case of (2) perhaps this should be a parameter all instruments have?

I added this parameter to deal with big data points caused by vanadium normalization. Yeah, I agree with you that it is called in multiple places and we need to make them consistent. This hard-coded ReplaceSpecialValues has been added recently by you. Why did you add it?

github-actions · 2023-07-17T09:19:55Z

👋 Hi, @MohamedAlmaki,

Conflicts have been detected against the base branch. Please rebase your branch against the base branch.

RichardWaiteSTFC

Looks good - thanks for this. Lets think about standardising the ReplaceSpecialValues thresholds in another PR!

MohamedAlmaki marked this pull request as draft April 3, 2023 15:43

AnthonyLim23 reviewed Apr 5, 2023

View reviewed changes

scripts/Diffraction/isis_powder/osiris.py Show resolved Hide resolved

AnthonyLim23 reviewed Apr 5, 2023

View reviewed changes

scripts/Diffraction/isis_powder/osiris.py Outdated Show resolved Hide resolved

AnthonyLim23 reviewed Apr 5, 2023

View reviewed changes

scripts/Diffraction/isis_powder/osiris.py Show resolved Hide resolved

AnthonyLim23 reviewed Apr 5, 2023

View reviewed changes

scripts/Diffraction/isis_powder/osiris.py Outdated Show resolved Hide resolved

AnthonyLim23 self-assigned this Apr 5, 2023

AnthonyLim23 added the ISIS Team: Spectroscopy Issue and pull requests managed by the Spectroscopy subteam at ISIS label Apr 5, 2023

AnthonyLim23 added this to the Release 6.7 milestone Apr 5, 2023

MohamedAlmaki force-pushed the 34311_add_absorbtion_corrections branch from 9ec59e5 to 3002a73 Compare April 5, 2023 14:11

AnthonyLim23 removed their assignment Apr 6, 2023

AnthonyLim23 reviewed May 2, 2023

View reviewed changes

MohamedAlmaki marked this pull request as ready for review May 2, 2023 15:07

MohamedAlmaki modified the milestones: Release 6.7, Release 6.8 May 15, 2023

AnthonyLim23 reviewed Jun 16, 2023

View reviewed changes

github-actions bot added the Has Conflicts Used by the bot to label pull requests that have conflicts label Jun 21, 2023

MohamedAlmaki force-pushed the 34311_add_absorbtion_corrections branch from d5c63ca to 2ce8ffc Compare June 21, 2023 10:02

github-actions bot removed the Has Conflicts Used by the bot to label pull requests that have conflicts label Jun 21, 2023

AnthonyLim23 previously approved these changes Jun 22, 2023

View reviewed changes

AnthonyLim23 previously approved these changes Jun 29, 2023

View reviewed changes

RichardWaiteSTFC requested changes Jul 6, 2023

View reviewed changes

github-actions bot added the Has Conflicts Used by the bot to label pull requests that have conflicts label Jul 17, 2023

MohamedAlmaki added 11 commits July 21, 2023 11:40

Add absorbtion corrections

95fd929

Fix error in workspaces output

475a367

Fix absorption correction and add big threshold option

d4ceee5

Fix system tests

c9ae19b

Update docstrings

8b53a3c

Add event per point for simple absorption correction

471899f

Add system tests for absorption correction methods

964ab70

Update mapping yaml reference hash

4919648

Refactor system tests to use a common interface

cc2202b

Increase error tolerance for paalman pings absorption test

f856e1f

Turn on tolerance is real error

4cbf00c

MohamedAlmaki dismissed AnthonyLim23’s stale review via 4cbf00c July 21, 2023 10:41

MohamedAlmaki force-pushed the 34311_add_absorbtion_corrections branch from 329ee7e to 4cbf00c Compare July 21, 2023 10:41

github-actions bot removed the Has Conflicts Used by the bot to label pull requests that have conflicts label Jul 21, 2023

Remove cut off parameter

a591b65

RichardWaiteSTFC approved these changes Jul 21, 2023

View reviewed changes

gemmaguest merged commit a6a676b into main Jul 28, 2023
8 checks passed

gemmaguest deleted the 34311_add_absorbtion_corrections branch July 28, 2023 11:32

MohamedAlmaki mentioned this pull request Aug 15, 2023

Add multiple scattering correction to OSIRIS script #35927

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add absorbtion corrections to the OSIRIS scripts #35430

Add absorbtion corrections to the OSIRIS scripts #35430

MohamedAlmaki commented Apr 3, 2023 •

edited

Loading

SpencerHowells commented Apr 3, 2023

MohamedAlmaki commented Apr 4, 2023 •

edited

Loading

AnthonyLim23 commented Apr 6, 2023

AnthonyLim23 May 2, 2023

MohamedAlmaki May 2, 2023

AnthonyLim23 commented May 2, 2023

AnthonyLim23 Jun 16, 2023

MohamedAlmaki Jun 16, 2023

AnthonyLim23 Jun 16, 2023

MohamedAlmaki Jun 19, 2023

AnthonyLim23 Jun 19, 2023

MohamedAlmaki Jun 19, 2023

AnthonyLim23 Jun 19, 2023

MohamedAlmaki Jun 19, 2023

AnthonyLim23 commented Jun 20, 2023

MohamedAlmaki commented Jun 20, 2023

github-actions bot commented Jun 21, 2023

AnthonyLim23 left a comment

AnthonyLim23 left a comment

RichardWaiteSTFC left a comment

RichardWaiteSTFC Jul 6, 2023

MohamedAlmaki Jul 10, 2023

RichardWaiteSTFC Jul 6, 2023 •

edited

Loading

MohamedAlmaki Jul 10, 2023

github-actions bot commented Jul 17, 2023

RichardWaiteSTFC left a comment

	aligned_ws = mantid.ReplaceSpecialValues(
	InputWorkspace=aligned_ws,
	OutputWorkspace=aligned_ws.name(),
	NaNValue=0,
	InfinityValue=0,
	BigNumberThreshold=1e15,
	SmallNumberThreshold=-1e15,

Add absorbtion corrections to the OSIRIS scripts #35430

Add absorbtion corrections to the OSIRIS scripts #35430

Conversation

MohamedAlmaki commented Apr 3, 2023 • edited Loading

Reviewer

Code Review

Functional Tests

SpencerHowells commented Apr 3, 2023

MohamedAlmaki commented Apr 4, 2023 • edited Loading

AnthonyLim23 commented Apr 6, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AnthonyLim23 commented May 2, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AnthonyLim23 commented Jun 20, 2023

MohamedAlmaki commented Jun 20, 2023

github-actions bot commented Jun 21, 2023

AnthonyLim23 left a comment

Choose a reason for hiding this comment

AnthonyLim23 left a comment

Choose a reason for hiding this comment

RichardWaiteSTFC left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RichardWaiteSTFC Jul 6, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jul 17, 2023

RichardWaiteSTFC left a comment

Choose a reason for hiding this comment

MohamedAlmaki commented Apr 3, 2023 •

edited

Loading

MohamedAlmaki commented Apr 4, 2023 •

edited

Loading

RichardWaiteSTFC Jul 6, 2023 •

edited

Loading