Add wrapper algorithm to calculate polarization efficiencies #38996
Conversation
MialLewis
added
ISIS Team: LSS
MialLewis
force-pushed
the
add_wrapper_alg_to_calculate_pol_eff
branch
from
28ecf6e to
9b41539
Compare
There was a problem hiding this comment.
The algorithm is working fine, it's well structured and neat, and all test are passing.
Testing is well covered with unit and system tests.
I have some minor comments (mostly nitpicked) about documentation and couple other places.
The only issue I have seen is that the temporary workspaces from CreateTransmission algorithm are not deleted, i'm not sure if this is intentional? But if the algorithm is called repeated times, it can create a big chunk of repeated loaded runs that are hidden workspaces (this temporaries are deleted if the CreateTransmission algorithm is called individually)
...rk/PythonInterface/plugins/algorithms/WorkflowAlgorithms/ReflectometryISISCalculatePolEff.py
Outdated
Show resolved
Hide resolved
...rk/PythonInterface/plugins/algorithms/WorkflowAlgorithms/ReflectometryISISCalculatePolEff.py
Outdated
Show resolved
Hide resolved
| If a ``FloodWorkspace`` is provided then a flood correction is performed as part of :ref:`algm-ReflectometryISISCreateTransmission` for both magnetic and non-magnetic input runs. | ||
|
|
||
| If workspace indices are provided to the either of the ``BackgroundProcessingInstructions`` or ``MagBackgroundProcessingInstructions`` properties then a background subtraction is performed as part of :ref:`algm-ReflectometryISISCreateTransmission` for the relevant input runs. | ||
|
|
There was a problem hiding this comment.
Could we add two short sentences indicating where the missing properties are used: IOMonitorIndex, MonitorIntegrationWavelengthMin, MonitorIntegrationWavelengthMax are used to create transmission workspaces.
Filppers, InputPolariserEfficiency, InputAnalyserEfficiency , IncludeDiagnosticsOutput passed to PolarizationEfficienciesWildes
There was a problem hiding this comment.
What I've decided to do here is that if a property is novel to this algorithm then I've added a short description here. Otherwise I just link to the existing documentation for the child algorithms.
| eff_args.update(self._populate_args_dict(_ALGS["EFF_ALG"])) | ||
| return eff_args | ||
|
|
||
| def _set_output_properties(self, join_ws, eff_output): |
There was a problem hiding this comment.
When I call the algorithm from the python editor, the name of the workspace in the ADS is not the same as the one in the editor but the default value:
If I call the algorithm from the editor, but setting IncludeDiagnogsticOutput=True, then the name of the output ws on the ads will be pol_eff.
Maybe this is not something related to this pr but the python interface, do you know why this happens?
There was a problem hiding this comment.
Thanks for spotting this. The python interface (simpleapi) passes the lvalue (pol_eff in this case) into the algorithm as an argument. This lvalue gets assigned as the output name of the first output workspace property.
When IncludeDiagnogsticOutput=True is set, the first workspace property changes to the diagnostic workspace Rho, as opposed to the final OutputWorkspace of this algorithm as one would expect.
To stop this I've had to reorder the algorithm properties so that OutputWorkspace is always the first output workspace property.
...rk/PythonInterface/plugins/algorithms/WorkflowAlgorithms/ReflectometryISISCalculatePolEff.py
Outdated
Show resolved
Hide resolved
I thought I checked that the clean up worked, but I must have accidently turned off show hidden workspaces! I think this is more a problem with EDIT: See issue here: #39041 |
Description of work
This PR adds the
ReflectometryISISCalculatePolEffalgorithm.This algorithm is a wrapper around
ReflectometryISISCreateTransmission,PolarizationEfficienciesWildesandJoinISISPolarizationEfficiencies.The files needed to test this PR are in the system test data:
POLREF32130,POLREF32132POLREF32131,POLREF32133POLREF_Flood_TOF_single_bin.nx5Please test with a variety of different inputs, example inputs can be seem on the system test.
Status: Open.
#38470
Further detail of work
To test:
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/gatekeeperswill 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.