Skip to content

Commit

Permalink
Don't return previous values when alg run twice
Browse files Browse the repository at this point in the history
The algorithm uses private member variables that need to be cleared to
prevent previously calculated values being returned if an instance of
the algorithm is run twice. In addition, the algorithm provides some
optional diagnostic output properties that we provide default values for. If the algorithm is being run as a child
then these can incorrectly remain populated with previous values if we do not clear them when they are not required. It seems that setting the property value to it's current value (to deliberately retain any edits the user has made to that) is sufficient to clear the associated output workspace.
  • Loading branch information
rbauststfc committed Aug 20, 2024
1 parent e457d7c commit b9643f4
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,16 @@ class MANTID_ALGORITHMS_DLL PolarizationEfficienciesWildes : public API::Algorit
/// Solve for the unknown efficiency from either (2p-1) or (2a-1) using the relationship Phi = (2p-1)(2a-1)
MatrixWorkspace_sptr solveUnknownEfficiencyFromTXMO(const MatrixWorkspace_sptr &wsTXMO);

/// Set the algorithm outputs
/// Set the algorithm outputs
void setOutputs();

/// Clear the values for all the algorithm member variables
void resetMemberVariables();

/// Sets the property value to its current value. For output workspace properties this will clear any workspaces being
/// held by the property
void resetPropertyValue(const std::string &propertyName);

MatrixWorkspace_sptr m_wsFp = nullptr;
MatrixWorkspace_sptr m_wsFa = nullptr;
MatrixWorkspace_sptr m_wsPhi = nullptr;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,9 @@ void PolarizationEfficienciesWildes::exec() {

progress.report(8, "Setting algorithm outputs");
setOutputs();

// Ensure that we don't carry over values from a previous run if an instance of this algorithm is run twice
resetMemberVariables();
}

namespace {
Expand Down Expand Up @@ -395,12 +398,37 @@ void PolarizationEfficienciesWildes::setOutputs() {
if (m_wsP != nullptr) {
const auto wsTPMO = (2 * m_wsP) - 1;
setProperty(PropNames::OUTPUT_TPMO_WS, wsTPMO);
} else if (isChild()) {
// Clear diagnostic output property that may have been populated in a previous run as a child algorithm
resetPropertyValue(PropNames::OUTPUT_TPMO_WS);
}

if (m_wsA != nullptr) {
const auto wsTAMO = (2 * m_wsA) - 1;
setProperty(PropNames::OUTPUT_TAMO_WS, wsTAMO);
} else if (isChild()) {
// Clear diagnostic output property that may have been populated in a previous run as a child algorithm
resetPropertyValue(PropNames::OUTPUT_TAMO_WS);
}
} else if (isChild()) {
// Clear diagnostic output properties that may have been populated in a previous run as a child algorithm
resetPropertyValue(PropNames::OUTPUT_PHI_WS);
resetPropertyValue(PropNames::OUTPUT_RHO_WS);
resetPropertyValue(PropNames::OUTPUT_ALPHA_WS);
resetPropertyValue(PropNames::OUTPUT_TPMO_WS);
resetPropertyValue(PropNames::OUTPUT_TAMO_WS);
}
}

void PolarizationEfficienciesWildes::resetMemberVariables() {
m_wsFp = nullptr;
m_wsFa = nullptr;
m_wsPhi = nullptr;
m_wsP = nullptr;
m_wsA = nullptr;
}

void PolarizationEfficienciesWildes::resetPropertyValue(const std::string &propertyName) {
setPropertyValue(propertyName, getPropertyValue(propertyName));
}
} // namespace Mantid::Algorithms
Original file line number Diff line number Diff line change
Expand Up @@ -411,6 +411,14 @@ class PolarizationEfficienciesWildesTest : public CxxTest::TestSuite {
runTestInputEfficiencyWorkspaceNotOverwrittenWhenSetAsOutput(InputPropNames::A_EFF_WS, OutputPropNames::A_EFF_WS);
}

void test_algorithm_clears_optional_outputs_on_second_run_with_same_include_diagnostics() {
runTestOutputWorkspacesSetCorrectlyForMultipleRuns(true);
}

void test_algorithm_clears_optional_outputs_on_second_run_with_different_include_diagnostics() {
runTestOutputWorkspacesSetCorrectlyForMultipleRuns(false);
}

private:
const std::vector<double> NON_MAG_Y_VALS = {12.0, 1.0, 2.0, 10.0};
const std::vector<double> MAG_Y_VALS = {6.0, 0.2, 0.3, 1.0};
Expand Down Expand Up @@ -639,4 +647,36 @@ class PolarizationEfficienciesWildesTest : public CxxTest::TestSuite {
const MatrixWorkspace_sptr outEffWs = alg->getProperty(outputPropName);
TS_ASSERT(outEffWs != inEffWs);
}

void runTestOutputWorkspacesSetCorrectlyForMultipleRuns(const bool secondRunIncludeDiagnostics) {
// We need to make sure we don't get outputs from previous runs if the same instance of the algorithm is run twice,
// or is being run as a child algorithm.
const auto nonMagGrp = createNonMagWSGroup("nonMagWs");
const auto magGrp = createMagWSGroup("magWs");
const auto alg = createEfficiencyAlg(nonMagGrp, magGrp);

alg->setPropertyValue(OutputPropNames::P_EFF_WS, "pEff");
alg->setPropertyValue(OutputPropNames::A_EFF_WS, "aEff");
alg->setProperty(InputPropNames::INCLUDE_DIAGNOSTICS, true);
alg->execute();
checkOutputWorkspaceIsSet(alg, OutputPropNames::P_EFF_WS, true);
checkOutputWorkspaceIsSet(alg, OutputPropNames::A_EFF_WS, true);
checkOutputWorkspaceIsSet(alg, OutputPropNames::PHI_WS, true);
checkOutputWorkspaceIsSet(alg, OutputPropNames::ALPHA_WS, true);
checkOutputWorkspaceIsSet(alg, OutputPropNames::RHO_WS, true);
checkOutputWorkspaceIsSet(alg, OutputPropNames::TPMO_WS, true);
checkOutputWorkspaceIsSet(alg, OutputPropNames::TAMO_WS, true);

alg->setPropertyValue(OutputPropNames::P_EFF_WS, "");
alg->setPropertyValue(OutputPropNames::A_EFF_WS, "");
alg->setProperty(InputPropNames::INCLUDE_DIAGNOSTICS, secondRunIncludeDiagnostics);
alg->execute();
checkOutputWorkspaceIsSet(alg, OutputPropNames::P_EFF_WS, false);
checkOutputWorkspaceIsSet(alg, OutputPropNames::A_EFF_WS, false);
checkOutputWorkspaceIsSet(alg, OutputPropNames::PHI_WS, secondRunIncludeDiagnostics);
checkOutputWorkspaceIsSet(alg, OutputPropNames::ALPHA_WS, secondRunIncludeDiagnostics);
checkOutputWorkspaceIsSet(alg, OutputPropNames::RHO_WS, secondRunIncludeDiagnostics);
checkOutputWorkspaceIsSet(alg, OutputPropNames::TPMO_WS, false);
checkOutputWorkspaceIsSet(alg, OutputPropNames::TAMO_WS, false);
}
};

0 comments on commit b9643f4

Please sign in to comment.