Allow User-Configured Input Spin State Orders From The GUI & Parameter File #38783

cailafinn · 2025-02-03T18:25:08Z

Description of work

Summary of work

Adds two adjacent features to support the polarised reduction through the reflectometry interface.

Wildes

Allows the Flipper configuration (order of the spin states in the input workspace) to be given via the instrument parameter file. A default has been defined for POLREF.

Fredrikze

A rarer polarisation correction that would have different spin state orders on an experiment-to-experiment basis.
To facilitate this a new field has been added to the Experiment Settings tab of the ISIS Reflectometry Interface.
This field is then passed to the Fredrikze polarisation correction algorithm.
The reduction algorithm throws an error if the reduction expected to perform a Wildes correction but a value was entered for the Fredrikze order.

Fixes #37617

To test:

Open the ISIS Reflectometry Interface
Switch the Instrument to POLREF
Enter a run of 37125 and an angle of 2.3 on the runs tab.
Run this script:

eWs = CreateSampleWorkspace('Histogram', Function='User Defined', UserDefinedFunction='name=UserFunction,Formula=(1 + tanh(0.0733 * 12 * x * 0.2))/2',XUnit='Wavelength', xMin='1',XMax='8', BinWidth='1', NumBanks='1', BankPixelWidth='1')
# Wildes  {"P1", "P2", "F1", "F2"}
eff_wildes = JoinISISPolarizationEfficiencies(P1=eWs, P2=eWs, F1=eWs, F2=eWs )

# Fredrikze {"Pp", "Ap", "Rho", "Alpha"}
eff_fredrikze = JoinISISPolarizationEfficiencies(Pp=eWs, Ap=eWs, Rho=eWs, Alpha=eWs )

Swap to the experiment settings tab
Set the Polarization Corrections to workspace.

Wildes

Select the eff_wildes workspace from the Polarization Efficiencies dropdown
Run the reduction on the Runs tab. Given this run is a 2 workspace group, and the setting in the parameter file is for 4 workspaces, we expect this to fail. Change the WildesFlipperConfig in the POLREF_Parameter file from 00,01,10,11 to 0,1 to see it succeed.

Fredrikze

Select the eff_fredrikze workspace from the polarization efficiencies dropdown
Enter a fredrikze spin state order into the text entry box and run the reduction.
It should fail when four entries are given E.g pa,ap,pp,aa
It should succeed when two entries are given p, a

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

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?
Do the release notes conform to the release notes guide?

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.

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.

RE mantidproject#37617

This is needed for batch saving/loading and for project recovery. RE mantidproject#37617

adriazalvarez

I have followed the testing instructions, I found an issue with the validation of the flipper setup in the idf file that maybe we should look into in a separate issue.
Aside that, the code changes look good and the GUI is behaving as expected. Thanks for updating the testing instructions too.
I have added some other minor comments.

adriazalvarez · 2025-02-12T11:10:10Z

...ork/PythonInterface/plugins/algorithms/WorkflowAlgorithms/ReflectometryISISLoadAndProcess.py

+        self.declareProperty(
+            "FredrikzePolarizationSpinStateOrder",
+            "",
+            "The spin state order of the workspaces in the workspace group to be passed to "


The description for this property is very nice.
We could maybe extend the documentation page of the Reflectometry Reduction One Auto algorithm (adding couple of lines) being a bit more explicit on how the setting is added on the parameters file using the WildesFlipperConfig field?

adriazalvarez · 2025-02-12T11:11:36Z

Framework/Reflectometry/inc/MantidReflectometry/ReflectometryReductionOneAuto3.h

@@ -88,8 +88,9 @@ class MANTID_REFLECTOMETRY_DLL ReflectometryReductionOneAuto3 : public Reflectom
                                               const Mantid::Geometry::Instrument_const_sptr &instrument);
  std::string findPolarizationCorrectionMethod(const API::MatrixWorkspace_sptr &efficiencies);
  std::string findPolarizationCorrectionOption(const std::string &correctionMethod);
+  std::string getFredrikzeInputSpinStateOrder(std::string const &correctionMethod);


This file is mostly written in West const convention, would it be possible to change it here to make it more homogeneous ?

adriazalvarez · 2025-02-12T11:29:29Z

Framework/Reflectometry/test/ReflectometryReductionOneAuto3Test.h

+    // We're setting this to an invalid value to catch it on purpose later.
+    input_group[0]->instrumentParameters().addString(input_group[0]->getInstrument()->getComponentID(),
+                                                     "WildesFlipperConfig", "01,01,10");
+    auto efficiencies = createPolarizationEfficienciesWorkspace("Wildes");


I was going to mention that the setup for these algorithms could be clustered into a separate method, but it seems most of the tests in this suite are prepared within the test method, maybe we should open an issue to refactor this test suite and reduce duplicate lines.

adriazalvarez · 2025-02-12T11:37:23Z

qt/scientific_interfaces/ISISReflectometry/GUI/Experiment/QtExperimentView.cpp

+  return getText(*m_ui.polCorrFredrikzeSpinStateEdit);
+}
+
+void QtExperimentView::setFredrikzeSpinStateOrder(const std::string &spinStates) {


This signature here is west const and in the method declaration is east const . Could you make them the same constness?

adriazalvarez · 2025-02-12T11:39:13Z

qt/scientific_interfaces/ISISReflectometry/Reduction/PolarizationCorrections.h

@@ -47,14 +47,17 @@ inline std::string polarizationCorrectionTypeToString(PolarizationCorrectionType
 class MANTIDQT_ISISREFLECTOMETRY_DLL PolarizationCorrections {
 public:
  explicit PolarizationCorrections(PolarizationCorrectionType correctionType,
-                                   boost::optional<std::string> workspace = boost::none);
+                                   boost::optional<std::string> workspace = boost::none,


Would it be possible to substitute the boost::optionals in this file for the std::optionals?

adriazalvarez · 2025-02-12T13:11:26Z

instrument/POLREF_Parameters.xml

@@ -83,6 +83,10 @@
    <value val="0.972762,0.001828,-0.000261,0.0"/>
   </parameter>

+   <parameter name="WildesFlipperConfig" type="string">
+     <value val="00,01,10,11" />


See comment about the flippers , but currently this should be rewritten as :

Suggested change

<value val="00,01,10,11" />

<value val="00, 01, 10, 11" />

adriazalvarez · 2025-02-12T13:55:19Z

Framework/Reflectometry/src/ReflectometryReductionOneAuto3.cpp

@@ -56,8 +56,8 @@ void validate(const std::string &method) {
 namespace CorrectionOption {
 static const std::string PNR{"PNR"};
 static const std::string PA{"PA"};
-static const std::string FLIPPERS_NO_ANALYSER{"0, 1"};
-static const std::string FLIPPERS_FULL{"00, 01, 10, 11"};
+static const std::string DEFAULT_FLIPPERS_NO_ANALYSER{"0, 1"};


I have found a problem with the input of the wildes flipper setup. PolarizedEfficiencyCorr algorithm is validating against a list of strings, and the possible values for flipper configurations are adding a whitespace after the comma (as in here). But in other places of this pr, like the tests or the POLREF_parameters file the configuration is added as : "00,01,10,11" without whitespaces, which causes an error. This is not happening with fredrikze states because it uses a spin state validator.
We should maybe handle better the validation for flipper inputs so that users typing for example "0,1" don't get an error. But this maybe outside the scope of this pr. At least We should maybe add some more information in the documentation or the error message to let the user know what is the correct format.

adriazalvarez · 2025-02-12T13:55:59Z

Framework/Reflectometry/test/ReflectometryReductionOneAuto3Test.h

+    auto input_group = retrieveOutWS(name);
+    // We're setting this to an invalid value to catch it on purpose later.
+    input_group[0]->instrumentParameters().addString(input_group[0]->getInstrument()->getComponentID(),
+                                                     "WildesFlipperConfig", "00,11,01,10");


Suggested change

"WildesFlipperConfig", "00,11,01,10");

"WildesFlipperConfig", "00, 11, 01, 10");

cailafinn added Reflectometry Issues and pull requests related to reflectometry ISIS Team: LSS Issue and pull requests managed by the LSS subteam at ISIS labels Feb 3, 2025

cailafinn added this to the Release 6.13 milestone Feb 3, 2025

cailafinn force-pushed the 37617_flipper_config_in_refl_gui branch 2 times, most recently from 65bed42 to 72f3e53 Compare February 4, 2025 08:49

cailafinn added 8 commits February 4, 2025 13:25

Return spin states from parameter file for Wildes

597084f

RE mantidproject#37617

Add fredrikze spin state line-edit to gui

c0fb266

RE mantidproject#37617

Add getters and setters to view

0cae891

RE mantidproject#37617

Implement input disabling logic

6c2cc57

RE mantidproject#37617

Add spin state order to Pol Corr model

3a2ea9c

RE mantidproject#37617

Update model from presenter

37551a8

RE mantidproject#37617

Validate Fredrikze spinstate order at alg level

b871776

RE mantidproject#37617

Add documentation

f0adb1d

RE mantidproject#37617

cailafinn force-pushed the 37617_flipper_config_in_refl_gui branch from 72f3e53 to 0fa794a Compare February 4, 2025 13:27

Include new field in encoder and decoder

59cef31

This is needed for batch saving/loading and for project recovery. RE mantidproject#37617

cailafinn force-pushed the 37617_flipper_config_in_refl_gui branch from 0fa794a to 59cef31 Compare February 4, 2025 14:51

cailafinn marked this pull request as ready for review February 4, 2025 15:58

MialLewis requested a review from adriazalvarez February 5, 2025 09:49

adriazalvarez requested changes Feb 12, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow User-Configured Input Spin State Orders From The GUI & Parameter File #38783

Allow User-Configured Input Spin State Orders From The GUI & Parameter File #38783

cailafinn commented Feb 3, 2025 •

edited

Loading

adriazalvarez left a comment

adriazalvarez Feb 12, 2025

adriazalvarez Feb 12, 2025

adriazalvarez Feb 12, 2025

adriazalvarez Feb 12, 2025

adriazalvarez Feb 12, 2025

adriazalvarez Feb 12, 2025

adriazalvarez Feb 12, 2025

adriazalvarez Feb 12, 2025

	"WildesFlipperConfig", "00,11,01,10");
	"WildesFlipperConfig", "00, 11, 01, 10");

Allow User-Configured Input Spin State Orders From The GUI & Parameter File #38783

Are you sure you want to change the base?

Allow User-Configured Input Spin State Orders From The GUI & Parameter File #38783

Conversation

cailafinn commented Feb 3, 2025 • edited Loading

Description of work

Summary of work

Wildes

Fredrikze

To test:

Wildes

Fredrikze

Reviewer

Code Review

Functional Tests

Gatekeeper

adriazalvarez left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cailafinn commented Feb 3, 2025 •

edited

Loading