From 073888d0c01ecf9ba33d6bc42fc3ff3bd46fcda4 Mon Sep 17 00:00:00 2001 From: Mohammed Almakki Date: Wed, 1 Nov 2023 10:10:31 +0000 Subject: [PATCH 01/12] Fix bin edges axis in AppendSpectra --- Framework/Algorithms/src/AppendSpectra.cpp | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/Framework/Algorithms/src/AppendSpectra.cpp b/Framework/Algorithms/src/AppendSpectra.cpp index 6afe5fb4dc01..7a50aa753b31 100644 --- a/Framework/Algorithms/src/AppendSpectra.cpp +++ b/Framework/Algorithms/src/AppendSpectra.cpp @@ -5,6 +5,7 @@ // Institut Laue - Langevin & CSNS, Institute of High Energy Physics, CAS // SPDX - License - Identifier: GPL - 3.0 + #include "MantidAlgorithms/AppendSpectra.h" +#include "MantidAPI/BinEdgeAxis.h" #include "MantidAPI/CommonBinsValidator.h" #include "MantidAPI/NumericAxis.h" #include "MantidAPI/Run.h" @@ -106,7 +107,7 @@ void AppendSpectra::exec() { setProperty("OutputWorkspace", std::dynamic_pointer_cast(output)); } -/** If there is an overlap in spectrum numbers between ws1 and ws2, +/** If there is an overlap in spectrum numbers between ws1 and ws2 or the y axis is a bin edges axis, * then the spectrum numbers are reset as a simple 1-1 correspondence * with the workspace index. * @@ -124,25 +125,29 @@ void AppendSpectra::fixSpectrumNumbers(const MatrixWorkspace &ws1, const MatrixW specnum_t ws2max; getMinMax(ws2, ws2min, ws2max); - // is everything possibly ok? - if (ws2min > ws1max) + const int yAxisNum = 1; + auto outputYAxis = output.getAxis(yAxisNum); + const bool spectraAreNotOverlapping = ws2min > ws1max; + const bool notYBinEdgeAxis = dynamic_cast(outputYAxis) != nullptr; + if (spectraAreNotOverlapping && notYBinEdgeAxis) return; auto indexInfo = output.indexInfo(); indexInfo.setSpectrumNumbers(0, static_cast(output.getNumberHistograms() - 1)); output.setIndexInfo(indexInfo); - const int yAxisNum = 1; const auto yAxisWS1 = ws1.getAxis(yAxisNum); const auto yAxisWS2 = ws2.getAxis(yAxisNum); - auto outputYAxis = output.getAxis(yAxisNum); const auto ws1len = ws1.getNumberHistograms(); const bool isTextAxis = yAxisWS1->isText() && yAxisWS2->isText(); const bool isNumericAxis = yAxisWS1->isNumeric() && yAxisWS2->isNumeric(); + bool isBinEdgeAxis = + dynamic_cast(yAxisWS1) != nullptr && dynamic_cast(yAxisWS2) != nullptr; auto outputTextAxis = dynamic_cast(outputYAxis); - for (size_t i = 0; i < output.getNumberHistograms(); ++i) { + const auto ouputlen = output.getNumberHistograms() + (isBinEdgeAxis ? 1 : 0); + for (size_t i = 0; i < ouputlen; ++i) { if (isTextAxis) { // check if we're outside the spectra of the first workspace const std::string inputLabel = i < ws1len ? yAxisWS1->label(i) : yAxisWS2->label(i - ws1len); From 203662a27620d308e162c63f2a39d6f86629e356 Mon Sep 17 00:00:00 2001 From: Mohammed Almakki Date: Wed, 1 Nov 2023 13:15:08 +0000 Subject: [PATCH 02/12] Change copying axis into a parameter --- .../inc/MantidAlgorithms/AppendSpectra.h | 1 + Framework/Algorithms/src/AppendSpectra.cpp | 24 +++++++++++-------- .../kernel/src/Exports/LambdaValidator.cpp | 21 ++++++++++++++++ 3 files changed, 36 insertions(+), 10 deletions(-) create mode 100644 Framework/PythonInterface/mantid/kernel/src/Exports/LambdaValidator.cpp diff --git a/Framework/Algorithms/inc/MantidAlgorithms/AppendSpectra.h b/Framework/Algorithms/inc/MantidAlgorithms/AppendSpectra.h index 9460a3abc828..059ec2f81c3a 100644 --- a/Framework/Algorithms/inc/MantidAlgorithms/AppendSpectra.h +++ b/Framework/Algorithms/inc/MantidAlgorithms/AppendSpectra.h @@ -40,6 +40,7 @@ class MANTID_ALGORITHMS_DLL AppendSpectra : public WorkspaceJoiners { void fixSpectrumNumbers(const API::MatrixWorkspace &ws1, const API::MatrixWorkspace &ws2, API::MatrixWorkspace &output) override; void combineLogs(const API::Run &lhs, const API::Run &rhs, API::Run &ans); + void copyYAxis(const API::MatrixWorkspace &ws1, const API::MatrixWorkspace &ws2, API::MatrixWorkspace &output); }; } // namespace Algorithms diff --git a/Framework/Algorithms/src/AppendSpectra.cpp b/Framework/Algorithms/src/AppendSpectra.cpp index 7a50aa753b31..a24f719b7da8 100644 --- a/Framework/Algorithms/src/AppendSpectra.cpp +++ b/Framework/Algorithms/src/AppendSpectra.cpp @@ -50,6 +50,7 @@ void AppendSpectra::init() { "The name of the output workspace"); declareProperty("MergeLogs", false, "Whether to combine the logs of the two input workspaces"); + declareProperty("CopyNumericAxis", false, "Whether to copy y axis numberic values from old workspaces"); } /** Execute the algorithm. @@ -125,17 +126,21 @@ void AppendSpectra::fixSpectrumNumbers(const MatrixWorkspace &ws1, const MatrixW specnum_t ws2max; getMinMax(ws2, ws2min, ws2max); - const int yAxisNum = 1; - auto outputYAxis = output.getAxis(yAxisNum); - const bool spectraAreNotOverlapping = ws2min > ws1max; - const bool notYBinEdgeAxis = dynamic_cast(outputYAxis) != nullptr; - if (spectraAreNotOverlapping && notYBinEdgeAxis) - return; + auto isNumeric = output.getAxis(1)->isNumeric(); + const bool copyNumericAxis = getProperty("CopyNumericAxis"); - auto indexInfo = output.indexInfo(); - indexInfo.setSpectrumNumbers(0, static_cast(output.getNumberHistograms() - 1)); - output.setIndexInfo(indexInfo); + if (ws2min < ws1max) { + auto indexInfo = output.indexInfo(); + indexInfo.setSpectrumNumbers(0, static_cast(output.getNumberHistograms() - 1)); + output.setIndexInfo(indexInfo); + copyYAxis(ws1, ws2, output); + } else if (isNumeric && copyNumericAxis) { + copyYAxis(ws1, ws2, output); + } +} + +void AppendSpectra::copyYAxis(const MatrixWorkspace &ws1, const MatrixWorkspace &ws2, MatrixWorkspace &output) { const auto yAxisWS1 = ws1.getAxis(yAxisNum); const auto yAxisWS2 = ws2.getAxis(yAxisNum); const auto ws1len = ws1.getNumberHistograms(); @@ -144,7 +149,6 @@ void AppendSpectra::fixSpectrumNumbers(const MatrixWorkspace &ws1, const MatrixW const bool isNumericAxis = yAxisWS1->isNumeric() && yAxisWS2->isNumeric(); bool isBinEdgeAxis = dynamic_cast(yAxisWS1) != nullptr && dynamic_cast(yAxisWS2) != nullptr; - auto outputTextAxis = dynamic_cast(outputYAxis); const auto ouputlen = output.getNumberHistograms() + (isBinEdgeAxis ? 1 : 0); for (size_t i = 0; i < ouputlen; ++i) { diff --git a/Framework/PythonInterface/mantid/kernel/src/Exports/LambdaValidator.cpp b/Framework/PythonInterface/mantid/kernel/src/Exports/LambdaValidator.cpp new file mode 100644 index 000000000000..0696ad8bc5ce --- /dev/null +++ b/Framework/PythonInterface/mantid/kernel/src/Exports/LambdaValidator.cpp @@ -0,0 +1,21 @@ +// Mantid Repository : https://github.com/mantidproject/mantid +// +// Copyright © 2023 ISIS Rutherford Appleton Laboratory UKRI, +// NScD Oak Ridge National Laboratory, European Spallation Source, +// Institut Laue - Langevin & CSNS, Institute of High Energy Physics, CAS +// SPDX - License - Identifier: GPL - 3.0 + +#include "MantidKernel/LambdaValidator.h" +#include "MantidPythonInterface/core/GetPointer.h" +#include +#include + +using Mantid::Kernel::LambdaValidator; +using namespace boost::python; + +GET_POINTER_SPECIALIZATION(LambdaValidator) + +void export_LambdaValidator() { + register_ptr_to_python>(); + + class_("IValidator", no_init); +} From 40523c90cb77206ccae405bbc2eb50859baf9e34 Mon Sep 17 00:00:00 2001 From: Mohammed Almakki Date: Thu, 2 Nov 2023 10:18:34 +0000 Subject: [PATCH 03/12] Fix errors --- Framework/Algorithms/src/AppendSpectra.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Framework/Algorithms/src/AppendSpectra.cpp b/Framework/Algorithms/src/AppendSpectra.cpp index a24f719b7da8..f4c8c13b96f8 100644 --- a/Framework/Algorithms/src/AppendSpectra.cpp +++ b/Framework/Algorithms/src/AppendSpectra.cpp @@ -141,8 +141,10 @@ void AppendSpectra::fixSpectrumNumbers(const MatrixWorkspace &ws1, const MatrixW } void AppendSpectra::copyYAxis(const MatrixWorkspace &ws1, const MatrixWorkspace &ws2, MatrixWorkspace &output) { + const auto yAxisNum = 1; const auto yAxisWS1 = ws1.getAxis(yAxisNum); const auto yAxisWS2 = ws2.getAxis(yAxisNum); + const auto outputYAxis = output.getAxis(yAxisNum); const auto ws1len = ws1.getNumberHistograms(); const bool isTextAxis = yAxisWS1->isText() && yAxisWS2->isText(); From 485edf7a01372e4e55122f4c9b9f0e93f9f7c76a Mon Sep 17 00:00:00 2001 From: Mohammed Almakki Date: Fri, 10 Nov 2023 18:16:53 +0000 Subject: [PATCH 04/12] Change the option to include all y axis types --- .../inc/MantidAlgorithms/AppendSpectra.h | 3 ++- Framework/Algorithms/src/AppendSpectra.cpp | 17 ++++++++--------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/Framework/Algorithms/inc/MantidAlgorithms/AppendSpectra.h b/Framework/Algorithms/inc/MantidAlgorithms/AppendSpectra.h index 059ec2f81c3a..d000ce9e5ba8 100644 --- a/Framework/Algorithms/inc/MantidAlgorithms/AppendSpectra.h +++ b/Framework/Algorithms/inc/MantidAlgorithms/AppendSpectra.h @@ -40,7 +40,8 @@ class MANTID_ALGORITHMS_DLL AppendSpectra : public WorkspaceJoiners { void fixSpectrumNumbers(const API::MatrixWorkspace &ws1, const API::MatrixWorkspace &ws2, API::MatrixWorkspace &output) override; void combineLogs(const API::Run &lhs, const API::Run &rhs, API::Run &ans); - void copyYAxis(const API::MatrixWorkspace &ws1, const API::MatrixWorkspace &ws2, API::MatrixWorkspace &output); + void appendYAxisLabels(const API::MatrixWorkspace &ws1, const API::MatrixWorkspace &ws2, + API::MatrixWorkspace &output); }; } // namespace Algorithms diff --git a/Framework/Algorithms/src/AppendSpectra.cpp b/Framework/Algorithms/src/AppendSpectra.cpp index f4c8c13b96f8..d8ef49d85ee1 100644 --- a/Framework/Algorithms/src/AppendSpectra.cpp +++ b/Framework/Algorithms/src/AppendSpectra.cpp @@ -50,7 +50,8 @@ void AppendSpectra::init() { "The name of the output workspace"); declareProperty("MergeLogs", false, "Whether to combine the logs of the two input workspaces"); - declareProperty("CopyNumericAxis", false, "Whether to copy y axis numberic values from old workspaces"); + declareProperty("AppendYAxisLabels", false, + "Whether to append y axis labels; this is done automatically if there is spectra overlap"); } /** Execute the algorithm. @@ -126,21 +127,19 @@ void AppendSpectra::fixSpectrumNumbers(const MatrixWorkspace &ws1, const MatrixW specnum_t ws2max; getMinMax(ws2, ws2min, ws2max); - auto isNumeric = output.getAxis(1)->isNumeric(); - const bool copyNumericAxis = getProperty("CopyNumericAxis"); - - if (ws2min < ws1max) { + const bool appendYAxis = getProperty("AppendYAxisLabels"); + if (ws2min <= ws1max) { auto indexInfo = output.indexInfo(); indexInfo.setSpectrumNumbers(0, static_cast(output.getNumberHistograms() - 1)); output.setIndexInfo(indexInfo); - copyYAxis(ws1, ws2, output); - } else if (isNumeric && copyNumericAxis) { - copyYAxis(ws1, ws2, output); + appendYAxisLabels(ws1, ws2, output); + } else if (appendYAxis) { + appendYAxisLabels(ws1, ws2, output); } } -void AppendSpectra::copyYAxis(const MatrixWorkspace &ws1, const MatrixWorkspace &ws2, MatrixWorkspace &output) { +void AppendSpectra::appendYAxisLabels(const MatrixWorkspace &ws1, const MatrixWorkspace &ws2, MatrixWorkspace &output) { const auto yAxisNum = 1; const auto yAxisWS1 = ws1.getAxis(yAxisNum); const auto yAxisWS2 = ws2.getAxis(yAxisNum); From fe60eb6d50875aa28c5fab6775fa969d5c2d1ced Mon Sep 17 00:00:00 2001 From: Mohammed Almakki Date: Fri, 10 Nov 2023 18:17:50 +0000 Subject: [PATCH 05/12] Refactor old tests and add tests for the new option --- Framework/Algorithms/test/AppendSpectraTest.h | 353 +++++++++++------- 1 file changed, 217 insertions(+), 136 deletions(-) diff --git a/Framework/Algorithms/test/AppendSpectraTest.h b/Framework/Algorithms/test/AppendSpectraTest.h index 8003c665d8bd..d7563fdc3ec1 100644 --- a/Framework/Algorithms/test/AppendSpectraTest.h +++ b/Framework/Algorithms/test/AppendSpectraTest.h @@ -9,6 +9,8 @@ #include "MantidAPI/AlgorithmManager.h" #include "MantidAPI/AnalysisDataService.h" #include "MantidAPI/Axis.h" +#include "MantidAPI/BinEdgeAxis.h" +#include "MantidAPI/NumericAxis.h" #include "MantidAPI/SpectrumInfo.h" #include "MantidAlgorithms/AppendSpectra.h" #include "MantidDataHandling/LoadRaw3.h" @@ -31,42 +33,16 @@ class AppendSpectraTest : public CxxTest::TestSuite { static AppendSpectraTest *createSuite() { return new AppendSpectraTest(); } static void destroySuite(AppendSpectraTest *suite) { delete suite; } - AppendSpectraTest() : ws1Name("ConjoinWorkspacesTest_grp1"), ws2Name("ConjoinWorkspacesTest_grp2") {} + AppendSpectraTest() {} - void setupWS() { - IAlgorithm *loader; - loader = new Mantid::DataHandling::LoadRaw3; - loader->initialize(); - loader->setPropertyValue("Filename", "OSI11886.raw"); - loader->setPropertyValue("OutputWorkspace", "top"); - loader->setPropertyValue("SpectrumMin", "1"); - loader->setPropertyValue("SpectrumMax", "10"); - TS_ASSERT_THROWS_NOTHING(loader->execute()); - TS_ASSERT(loader->isExecuted()); - delete loader; - - loader = new Mantid::DataHandling::LoadRaw3; - loader->initialize(); - loader->setPropertyValue("Filename", "OSI11886.raw"); - loader->setPropertyValue("OutputWorkspace", "bottom"); - loader->setPropertyValue("SpectrumMin", "11"); - loader->setPropertyValue("SpectrumMax", "25"); - TS_ASSERT_THROWS_NOTHING(loader->execute()); - TS_ASSERT(loader->isExecuted()); - delete loader; - } - - //---------------------------------------------------------------------------------------------- - void testExec() { - setupWS(); - - AppendSpectra alg; - if (!alg.isInitialized()) - alg.initialize(); + void test_algorithm_execution() { + std::string top = "top"; + std::string bottom = "bottom"; + setupRealWorkspaces(top, bottom); // Get the two input workspaces for later - MatrixWorkspace_sptr in1 = AnalysisDataService::Instance().retrieveWS("top"); - MatrixWorkspace_sptr in2 = AnalysisDataService::Instance().retrieveWS("bottom"); + MatrixWorkspace_sptr in1 = AnalysisDataService::Instance().retrieveWS(top); + MatrixWorkspace_sptr in2 = AnalysisDataService::Instance().retrieveWS(bottom); // Mask a spectrum and check it is carried over const size_t maskTop(5), maskBottom(10); @@ -76,14 +52,11 @@ class AppendSpectraTest : public CxxTest::TestSuite { in2->mutableSpectrumInfo().setMasked(maskBottom, true); // Now it should succeed - TS_ASSERT_THROWS_NOTHING(alg.setPropertyValue("InputWorkspace1", "top")); - TS_ASSERT_THROWS_NOTHING(alg.setPropertyValue("InputWorkspace2", "bottom")); - TS_ASSERT_THROWS_NOTHING(alg.setPropertyValue("OutputWorkspace", "top")); - TS_ASSERT_THROWS_NOTHING(alg.execute()); - TS_ASSERT(alg.isExecuted()); + auto appendSpectra = Mantid::API::AlgorithmManager::Instance().create("AppendSpectra"); + doAppendSpectraWithWorkspacesTest(appendSpectra, top, bottom, top); MatrixWorkspace_const_sptr output; - TS_ASSERT_THROWS_NOTHING(output = AnalysisDataService::Instance().retrieveWS("top")); + TS_ASSERT_THROWS_NOTHING(output = AnalysisDataService::Instance().retrieveWS(top)); TS_ASSERT_EQUALS(output->getNumberHistograms(), 25); // Check a few values TS_ASSERT_EQUALS(output->readX(0)[0], in1->readX(0)[0]); @@ -100,28 +73,21 @@ class AppendSpectraTest : public CxxTest::TestSuite { TS_ASSERT_EQUALS(output->spectrumInfo().isMasked(10 + maskBottom), true); } - //---------------------------------------------------------------------------------------------- - void testExecNumber() { - setupWS(); - - AppendSpectra alg; - if (!alg.isInitialized()) - alg.initialize(); + void test_algorithm_execution_with_multiple_appends() { + std::string top = "top"; + std::string bottom = "bottom"; + setupRealWorkspaces(top, bottom); // Get the two input workspaces for later - MatrixWorkspace_sptr in1 = AnalysisDataService::Instance().retrieveWS("top"); - MatrixWorkspace_sptr in2 = AnalysisDataService::Instance().retrieveWS("bottom"); + MatrixWorkspace_sptr in1 = AnalysisDataService::Instance().retrieveWS(top); + MatrixWorkspace_sptr in2 = AnalysisDataService::Instance().retrieveWS(bottom); // Now it should succeed - TS_ASSERT_THROWS_NOTHING(alg.setPropertyValue("InputWorkspace1", "top")); - TS_ASSERT_THROWS_NOTHING(alg.setPropertyValue("InputWorkspace2", "bottom")); - TS_ASSERT_THROWS_NOTHING(alg.setPropertyValue("OutputWorkspace", "top")); - TS_ASSERT_THROWS_NOTHING(alg.setProperty("Number", 2)); - TS_ASSERT_THROWS_NOTHING(alg.execute()); - TS_ASSERT(alg.isExecuted()); + auto appendSpectra = Mantid::API::AlgorithmManager::Instance().create("AppendSpectra"); + doAppendSpectraWithWorkspacesTest(appendSpectra, top, bottom, top, false, false, 2); MatrixWorkspace_const_sptr output; - TS_ASSERT_THROWS_NOTHING(output = AnalysisDataService::Instance().retrieveWS("top")); + TS_ASSERT_THROWS_NOTHING(output = AnalysisDataService::Instance().retrieveWS(top)); TS_ASSERT_EQUALS(output->getNumberHistograms(), 40); // Check a few values TS_ASSERT_EQUALS(output->readX(0)[0], in1->readX(0)[0]); @@ -140,8 +106,7 @@ class AppendSpectraTest : public CxxTest::TestSuite { TS_ASSERT_EQUALS(output->getAxis(1)->spectraNo(27), 27); } - //---------------------------------------------------------------------------------------------- - void testExecMismatchedWorkspaces() { + void test_two_workspaces_with_different_types() { MatrixWorkspace_sptr ews = WorkspaceCreationHelper::createEventWorkspace(10, 10); // Check it fails if mixing event workspaces and workspace 2Ds @@ -154,7 +119,7 @@ class AppendSpectraTest : public CxxTest::TestSuite { TS_ASSERT(!alg.isExecuted()); } - void testExecNonConstantBins() { + void test_two_workspaces_with_non_constant_bins() { AppendSpectra alg; alg.initialize(); TS_ASSERT_THROWS_NOTHING(alg.setProperty("InputWorkspace1", WorkspaceCreationHelper::create2DWorkspace(10, 10))); @@ -164,84 +129,29 @@ class AppendSpectraTest : public CxxTest::TestSuite { TS_ASSERT(!alg.isExecuted()); } - //---------------------------------------------------------------------------------------------- - void doTest(bool event, bool combineLogs = false, int number = 1) { - MatrixWorkspace_sptr ws1, ws2, out; - int numBins = 20; + void test_event_workspaces() { doTypeAndParamsTest(true); } - if (event) { - ws1 = WorkspaceCreationHelper::createEventWorkspace2(10, numBins); // 2 events per bin - ws2 = WorkspaceCreationHelper::createEventWorkspace2(5, numBins); - } else { - ws1 = WorkspaceCreationHelper::create2DWorkspace(10, numBins); - ws2 = WorkspaceCreationHelper::create2DWorkspace(5, numBins); - } - // Add instrument so detector IDs are valid and get copied. - InstrumentCreationHelper::addFullInstrumentToWorkspace(*ws1, false, false, ""); - InstrumentCreationHelper::addFullInstrumentToWorkspace(*ws2, false, false, ""); + void test_event_workspaces_with_merging_logs() { doTypeAndParamsTest(true, true); } - auto ws1Log = new TimeSeriesProperty("aLog"); - ws1Log->addValue(DateAndTime("2014-06-19T16:40:00"), "Hello"); - ws1->mutableRun().addLogData(ws1Log); + void test_event_workspaces_with_multiple_appends() { doTypeAndParamsTest(true, false, 3); } - auto ws2Log = new TimeSeriesProperty("aLog"); - ws2Log->addValue(DateAndTime("2014-06-19T16:40:10"), "World"); - ws2->mutableRun().addLogData(ws2Log); + void test_event_workspaces_with_merging_logs_and_multiple_appends() { (true, true, 3); } - AnalysisDataService::Instance().addOrReplace(ws1Name, ws1); - AnalysisDataService::Instance().addOrReplace(ws2Name, ws2); + void test_2D_workspaces() { doTypeAndParamsTest(false); } - AppendSpectra alg; - alg.initialize(); - TS_ASSERT_THROWS_NOTHING(alg.setPropertyValue("InputWorkspace1", ws1Name)); - TS_ASSERT_THROWS_NOTHING(alg.setPropertyValue("InputWorkspace2", ws2Name)); - TS_ASSERT_THROWS_NOTHING(alg.setPropertyValue("OutputWorkspace", ws1Name)); - TS_ASSERT_THROWS_NOTHING(alg.setProperty("Number", number)); - if (combineLogs) - alg.setProperty("MergeLogs", true); - TS_ASSERT_THROWS_NOTHING(alg.execute();) - TS_ASSERT(alg.isExecuted()); - - TS_ASSERT_THROWS_NOTHING( - out = std::dynamic_pointer_cast(AnalysisDataService::Instance().retrieve(ws1Name));) - TS_ASSERT(out); - if (!out) - return; - - TS_ASSERT_EQUALS(out->getNumberHistograms(), 10 + 5 * number); - TS_ASSERT_EQUALS(out->blocksize(), numBins); - - for (size_t wi = 0; wi < out->getNumberHistograms(); wi++) { - TS_ASSERT_EQUALS(out->getSpectrum(wi).getSpectrumNo(), specnum_t(wi)); - TS_ASSERT(!out->getSpectrum(wi).getDetectorIDs().empty()); - const auto &y = out->y(wi); - for (const auto value : y) - TS_ASSERT_DELTA(value, 2.0, 1e-5); - } - - if (combineLogs) { - TS_ASSERT_EQUALS(out->run().getTimeSeriesProperty("aLog")->size(), 2) - } else { - TS_ASSERT_EQUALS(out->run().getTimeSeriesProperty("aLog")->size(), 1) - } - } - - void test_events() { doTest(true); } + void test_2D_workspaces_with_merging_logs() { doTypeAndParamsTest(false, true); } - void test_number_events() { doTest(true, true, 3); } + void test_2D_workspaces_with_multiple_appends() { doTypeAndParamsTest(false, false, 3); } - void test_2D() { doTest(false); } + void test_2D_workspaces_with_merging_logs_and_multiple_appends() { doTypeAndParamsTest(false, true, 3); } - void test_events_mergeLogs() { doTest(true, true); } - - void test_2D_mergeLogs() { doTest(false, true); } - - void test_notEmptyTextAxis() { + void test_no_empty_text_axis() { const std::string inputWorkspace = "weRebinned"; const std::string outputWorkspace = "appended"; + auto appendSpectra = Mantid::API::AlgorithmManager::Instance().create("AppendSpectra"); createWorkspaceWithAxisAndLabel(inputWorkspace, "Text", "Text"); - doTestAppendSpectraWithWorkspaces(inputWorkspace, inputWorkspace, outputWorkspace); + doAppendSpectraWithWorkspacesTest(appendSpectra, inputWorkspace, inputWorkspace, outputWorkspace); MatrixWorkspace_const_sptr inputWS = AnalysisDataService::Instance().retrieveWS(inputWorkspace); MatrixWorkspace_const_sptr outputWS = AnalysisDataService::Instance().retrieveWS(outputWorkspace); @@ -256,12 +166,13 @@ class AppendSpectraTest : public CxxTest::TestSuite { } } - void test_emptyTextAxis() { + void test_empty_text_axis() { const std::string inputWorkspace = "weRebinned"; const std::string outputWorkspace = "appended"; + auto appendSpectra = Mantid::API::AlgorithmManager::Instance().create("AppendSpectra"); createWorkspaceWithAxisAndLabel(inputWorkspace, "Text", ""); - doTestAppendSpectraWithWorkspaces(inputWorkspace, inputWorkspace, outputWorkspace); + doAppendSpectraWithWorkspacesTest(appendSpectra, inputWorkspace, inputWorkspace, outputWorkspace); MatrixWorkspace_const_sptr inputWS = AnalysisDataService::Instance().retrieveWS(inputWorkspace); MatrixWorkspace_const_sptr outputWS = AnalysisDataService::Instance().retrieveWS(outputWorkspace); @@ -276,14 +187,15 @@ class AppendSpectraTest : public CxxTest::TestSuite { } } - void test_emptyAndNotEmptyTextAxis() { + void test_empty_and_not_empty_text_axis() { const std::string inputWorkspace1 = "weRebinned1"; const std::string inputWorkspace2 = "weRebinned2"; const std::string outputWorkspace = "appended"; + auto appendSpectra = Mantid::API::AlgorithmManager::Instance().create("AppendSpectra"); createWorkspaceWithAxisAndLabel(inputWorkspace1, "Text", "Text"); createWorkspaceWithAxisAndLabel(inputWorkspace2, "Text", ""); - doTestAppendSpectraWithWorkspaces(inputWorkspace1, inputWorkspace2, outputWorkspace); + doAppendSpectraWithWorkspacesTest(appendSpectra, inputWorkspace1, inputWorkspace2, outputWorkspace); MatrixWorkspace_const_sptr inputWS1 = AnalysisDataService::Instance().retrieveWS(inputWorkspace1); MatrixWorkspace_const_sptr inputWS2 = AnalysisDataService::Instance().retrieveWS(inputWorkspace2); @@ -309,12 +221,13 @@ class AppendSpectraTest : public CxxTest::TestSuite { } } - void test_numericAxis() { + void test_numeric_axis() { const std::string inputWorkspace = "weRebinned"; const std::string outputWorkspace = "appended"; + auto appendSpectra = Mantid::API::AlgorithmManager::Instance().create("AppendSpectra"); createWorkspaceWithAxisAndLabel(inputWorkspace, "Time", "1.0"); - doTestAppendSpectraWithWorkspaces(inputWorkspace, inputWorkspace, outputWorkspace); + doAppendSpectraWithWorkspacesTest(appendSpectra, inputWorkspace, inputWorkspace, outputWorkspace); MatrixWorkspace_const_sptr inputWS = AnalysisDataService::Instance().retrieveWS(inputWorkspace); MatrixWorkspace_const_sptr outputWS = AnalysisDataService::Instance().retrieveWS(outputWorkspace); @@ -329,14 +242,15 @@ class AppendSpectraTest : public CxxTest::TestSuite { } } - void test_differentNumericAxis() { + void test_different_numeric_axis() { const std::string inputWorkspace1 = "weRebinned1"; const std::string inputWorkspace2 = "weRebinned2"; const std::string outputWorkspace = "appended"; + auto appendSpectra = Mantid::API::AlgorithmManager::Instance().create("AppendSpectra"); createWorkspaceWithAxisAndLabel(inputWorkspace1, "Time", "1.0"); createWorkspaceWithAxisAndLabel(inputWorkspace2, "Time", "2.0"); - doTestAppendSpectraWithWorkspaces(inputWorkspace1, inputWorkspace2, outputWorkspace); + doAppendSpectraWithWorkspacesTest(appendSpectra, inputWorkspace1, inputWorkspace2, outputWorkspace); MatrixWorkspace_const_sptr inputWS1 = AnalysisDataService::Instance().retrieveWS(inputWorkspace1); MatrixWorkspace_const_sptr inputWS2 = AnalysisDataService::Instance().retrieveWS(inputWorkspace2); @@ -359,17 +273,164 @@ class AppendSpectraTest : public CxxTest::TestSuite { } } + void test_overlapped_append_y_axis_with_numeric_axis() { doAppendWorkspacesWithNumericAxisTest(true, false, true); } + + void test_overlapped_append_y_axis_with_numeric_axis_disabled() { + doAppendWorkspacesWithNumericAxisTest(false, false, true); + } + + void test_overlapped_append_y_axis_with_bin_edges_axis() { doAppendWorkspacesWithNumericAxisTest(true, true, true); } + + void test_overlapped_append_y_axis_with_bin_edges_axis_disabled() { + doAppendWorkspacesWithNumericAxisTest(false, true, true); + } + + void test_append_y_axis_with_numeric_axis() { doAppendWorkspacesWithNumericAxisTest(true, false, false); } + + void test_append_y_axis_with_numeric_axis_disabled() { doAppendWorkspacesWithNumericAxisTest(false, false, false); } + + void test_append_y_axis_with_bin_edges_axis() { doAppendWorkspacesWithNumericAxisTest(true, true, false); } + + void test_append_y_axis_with_bin_edges_axis_disabled() { doAppendWorkspacesWithNumericAxisTest(false, true, false); } + private: - void doTestAppendSpectraWithWorkspaces(const std::string &inputWorkspace1, const std::string &inputWorkspace2, - const std::string &outputWorkspace) { + void doTypeAndParamsTest(bool event, bool combineLogs = false, int number = 1) { + const std::string ws1Name = "AppendSpectraTest_grp1"; + const std::string ws2Name = "AppendSpectraTest_grp2"; + + MatrixWorkspace_sptr ws1, ws2, out; + int numBins = 20; + + if (event) { + ws1 = WorkspaceCreationHelper::createEventWorkspace2(10, numBins); // 2 events per bin + ws2 = WorkspaceCreationHelper::createEventWorkspace2(5, numBins); + } else { + ws1 = WorkspaceCreationHelper::create2DWorkspace(10, numBins); + ws2 = WorkspaceCreationHelper::create2DWorkspace(5, numBins); + } + // Add instrument so detector IDs are valid and get copied. + InstrumentCreationHelper::addFullInstrumentToWorkspace(*ws1, false, false, ""); + InstrumentCreationHelper::addFullInstrumentToWorkspace(*ws2, false, false, ""); + + auto ws1Log = new TimeSeriesProperty("aLog"); + ws1Log->addValue(DateAndTime("2014-06-19T16:40:00"), "Hello"); + ws1->mutableRun().addLogData(ws1Log); + + auto ws2Log = new TimeSeriesProperty("aLog"); + ws2Log->addValue(DateAndTime("2014-06-19T16:40:10"), "World"); + ws2->mutableRun().addLogData(ws2Log); + + AnalysisDataService::Instance().addOrReplace(ws1Name, ws1); + AnalysisDataService::Instance().addOrReplace(ws2Name, ws2); + auto appendSpectra = Mantid::API::AlgorithmManager::Instance().create("AppendSpectra"); + doAppendSpectraWithWorkspacesTest(appendSpectra, ws1Name, ws2Name, ws1Name, false, combineLogs, number); + + TS_ASSERT_THROWS_NOTHING( + out = std::dynamic_pointer_cast(AnalysisDataService::Instance().retrieve(ws1Name));) + TS_ASSERT(out); + if (!out) + return; + + TS_ASSERT_EQUALS(out->getNumberHistograms(), 10 + 5 * number); + TS_ASSERT_EQUALS(out->blocksize(), numBins); + + for (size_t wi = 0; wi < out->getNumberHistograms(); wi++) { + TS_ASSERT_EQUALS(out->getSpectrum(wi).getSpectrumNo(), specnum_t(wi)); + TS_ASSERT(!out->getSpectrum(wi).getDetectorIDs().empty()); + const auto &y = out->y(wi); + for (const auto value : y) + TS_ASSERT_DELTA(value, 2.0, 1e-5); + } + + if (combineLogs) { + TS_ASSERT_EQUALS(out->run().getTimeSeriesProperty("aLog")->size(), 2) + } else { + TS_ASSERT_EQUALS(out->run().getTimeSeriesProperty("aLog")->size(), 1) + } + } + + void doAppendSpectraWithWorkspacesTest(IAlgorithm_sptr appendSpectra, const std::string &inputWorkspace1, + const std::string &inputWorkspace2, const std::string &outputWorkspace, + const bool appendYAxis = false, const bool combineLogs = false, + const int number = 1) { + if (!appendSpectra->isInitialized()) + appendSpectra->initialize(); TS_ASSERT_THROWS_NOTHING(appendSpectra->setRethrows(true)); TS_ASSERT_THROWS_NOTHING(appendSpectra->setProperty("InputWorkspace1", inputWorkspace1)); TS_ASSERT_THROWS_NOTHING(appendSpectra->setProperty("InputWorkspace2", inputWorkspace2)); + TS_ASSERT_THROWS_NOTHING(appendSpectra->setProperty("AppendYAxisLabels", appendYAxis)); + TS_ASSERT_THROWS_NOTHING(appendSpectra->setProperty("Number", number)); + TS_ASSERT_THROWS_NOTHING(appendSpectra->setProperty("MergeLogs", combineLogs)); TS_ASSERT_THROWS_NOTHING(appendSpectra->setProperty("OutputWorkspace", outputWorkspace)); TS_ASSERT_THROWS_NOTHING(appendSpectra->execute()); TS_ASSERT(appendSpectra->isExecuted()); } + + void doAppendWorkspacesWithNumericAxisTest(bool appendYAxis, bool isBinEdgeAxis, bool overlapped) { + std::string inputWorkspace2; + std::string inputWorkspace1; + std::string outputWorkspace = "appended"; + + if (overlapped) { + inputWorkspace1 = "weRebinned1"; + inputWorkspace2 = "weRebinned2"; + createWorkspaceWithAxisAndLabel(inputWorkspace1, "Time", "1.0"); + createWorkspaceWithAxisAndLabel(inputWorkspace2, "Time", "1.0"); + } else { + inputWorkspace1 = "top"; + inputWorkspace2 = "bottom"; + setupRealWorkspaces(inputWorkspace1, inputWorkspace2); + } + + MatrixWorkspace_sptr inputWS1 = AnalysisDataService::Instance().retrieveWS(inputWorkspace1); + MatrixWorkspace_sptr inputWS2 = AnalysisDataService::Instance().retrieveWS(inputWorkspace2); + + appendNumericAxis(inputWS1, inputWS2, isBinEdgeAxis); + + auto appendSpectra = Mantid::API::AlgorithmManager::Instance().create("AppendSpectra"); + doAppendSpectraWithWorkspacesTest(appendSpectra, inputWorkspace1, inputWorkspace2, outputWorkspace, appendYAxis); + + bool isNotOverlappedAndDisabledAppend = !appendYAxis && !overlapped; + MatrixWorkspace_const_sptr outputWS = AnalysisDataService::Instance().retrieveWS(outputWorkspace); + auto outputAxis = outputWS->getAxis(1); + for (size_t i = 1; i < outputWS->getNumberHistograms(); ++i) { + TS_ASSERT_EQUALS(outputAxis->getValue(i), isNotOverlappedAndDisabledAppend ? 0 : i); + } + } + + void appendNumericAxis(MatrixWorkspace_sptr ws1, MatrixWorkspace_sptr ws2, const bool isBinEdges = false) { + const std::size_t ws1num = ws1->getNumberHistograms() + (isBinEdges ? 1 : 0); + const std::size_t ws2num = ws2->getNumberHistograms() + (isBinEdges ? 1 : 0); + + std::vector ws1Values(ws1num); + std::vector ws2Values(ws2num); + + size_t i, j; + + for (i = 0; i < ws1num; ++i) { + ws1Values[i] = i; + } + + for (j = i; j < (ws1num + ws2num); j++) { + ws2Values[j - ws1num] = j - (isBinEdges ? 1 : 0); + } + + std::unique_ptr ws1Axis; + std::unique_ptr ws2Axis; + + if (isBinEdges) { + ws1Axis = std::make_unique(ws1Values); + ws2Axis = std::make_unique(ws2Values); + } else { + ws1Axis = std::make_unique(ws1Values); + ws2Axis = std::make_unique(ws2Values); + } + + ws1->replaceAxis(1, std::move(ws1Axis)); + ws2->replaceAxis(1, std::move(ws2Axis)); + } + /** Creates a 2D workspace with 5 histograms */ void createWorkspaceWithAxisAndLabel(const std::string &outputName, const std::string &axisType, @@ -408,6 +469,26 @@ class AppendSpectraTest : public CxxTest::TestSuite { TS_ASSERT(rebin->isExecuted()); } - const std::string ws1Name; - const std::string ws2Name; + void setupRealWorkspaces(std::string ws1, std::string ws2) { + IAlgorithm *loader; + loader = new Mantid::DataHandling::LoadRaw3; + loader->initialize(); + loader->setPropertyValue("Filename", "OSI11886.raw"); + loader->setPropertyValue("OutputWorkspace", ws1); + loader->setPropertyValue("SpectrumMin", "1"); + loader->setPropertyValue("SpectrumMax", "10"); + TS_ASSERT_THROWS_NOTHING(loader->execute()); + TS_ASSERT(loader->isExecuted()); + delete loader; + + loader = new Mantid::DataHandling::LoadRaw3; + loader->initialize(); + loader->setPropertyValue("Filename", "OSI11886.raw"); + loader->setPropertyValue("OutputWorkspace", ws2); + loader->setPropertyValue("SpectrumMin", "11"); + loader->setPropertyValue("SpectrumMax", "25"); + TS_ASSERT_THROWS_NOTHING(loader->execute()); + TS_ASSERT(loader->isExecuted()); + delete loader; + } }; From c5aff63f24a20212ae6668f7b494f033fd8faa64 Mon Sep 17 00:00:00 2001 From: Mohammed Almakki Date: Fri, 10 Nov 2023 18:20:14 +0000 Subject: [PATCH 06/12] Add docs and release note --- docs/source/algorithms/AppendSpectra-v1.rst | 9 ++++++++- .../v6.9.0/Framework/Algorithms/New_features/36345.rst | 1 + 2 files changed, 9 insertions(+), 1 deletion(-) create mode 100644 docs/source/release/v6.9.0/Framework/Algorithms/New_features/36345.rst diff --git a/docs/source/algorithms/AppendSpectra-v1.rst b/docs/source/algorithms/AppendSpectra-v1.rst index 9a2fa0e1bb8f..7ff749361eb1 100644 --- a/docs/source/algorithms/AppendSpectra-v1.rst +++ b/docs/source/algorithms/AppendSpectra-v1.rst @@ -39,7 +39,14 @@ Spectrum Numbers If there is an overlap in the spectrum numbers of both inputs, then the output workspace will have its spectrum numbers reset starting at 0 and -increasing by 1 for each spectrum. +increasing by 1 for each spectrum. In addition, the y-axis value will be copied +from previous workspaces in order of the first workspace then the second workspace. + +Note that when spectra numbers do not overlap, +it doesn't automatically imply that y-axis values are carried over from previous workspaces. +To address this, use the 'AppendYAxisLabels' option. +This will combine y-axis values from two input workspaces into the new output workspace, +arranging them in the order of the first workspace followed by the second. .. seealso:: :ref:`algm-ConjoinWorkspaces` for joining parts of the same workspace. diff --git a/docs/source/release/v6.9.0/Framework/Algorithms/New_features/36345.rst b/docs/source/release/v6.9.0/Framework/Algorithms/New_features/36345.rst new file mode 100644 index 000000000000..36917f055ef6 --- /dev/null +++ b/docs/source/release/v6.9.0/Framework/Algorithms/New_features/36345.rst @@ -0,0 +1 @@ +- Add a new option (AppendYAxisLabels) to :ref:`AppendSpectra ` to append y-axis values from input workspaces to the input workspace \ No newline at end of file From 84aad382912a6d3bde9385c0d1a9b93f1dc60146 Mon Sep 17 00:00:00 2001 From: Mohammed Almakki Date: Fri, 10 Nov 2023 18:21:35 +0000 Subject: [PATCH 07/12] Fix grouping spectra with bin edges axis (sqw workspaces) --- .../Manipulation/InelasticDataManipulationElwinTabModel.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/qt/scientific_interfaces/Inelastic/Manipulation/InelasticDataManipulationElwinTabModel.cpp b/qt/scientific_interfaces/Inelastic/Manipulation/InelasticDataManipulationElwinTabModel.cpp index 34350aafc7d9..aae0b0afb871 100644 --- a/qt/scientific_interfaces/Inelastic/Manipulation/InelasticDataManipulationElwinTabModel.cpp +++ b/qt/scientific_interfaces/Inelastic/Manipulation/InelasticDataManipulationElwinTabModel.cpp @@ -110,6 +110,7 @@ std::string InelasticDataManipulationElwinTabModel::createGroupedWorkspaces(Matr auto appendSpectra = AlgorithmManager::Instance().create("AppendSpectra"); appendSpectra->setProperty("InputWorkspace1", workspace->getName() + "_extracted_spectra"); appendSpectra->setProperty("InputWorkspace2", "specWSnext"); + appendSpectra->setProperty("AppendYAxisLabels", true); appendSpectra->setProperty("OutputWorkspace", workspace->getName() + "_extracted_spectra"); appendSpectra->execute(); } From 0c4f9663d3af8e2738ec41013df10f32acd08dc8 Mon Sep 17 00:00:00 2001 From: Mohammed Almakki Date: Mon, 13 Nov 2023 12:53:05 +0000 Subject: [PATCH 08/12] Fix cpp warnnings and a typo in release note --- Framework/Algorithms/test/AppendSpectraTest.h | 6 +++--- .../v6.9.0/Framework/Algorithms/New_features/36345.rst | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Framework/Algorithms/test/AppendSpectraTest.h b/Framework/Algorithms/test/AppendSpectraTest.h index d7563fdc3ec1..47bbc0831bbf 100644 --- a/Framework/Algorithms/test/AppendSpectraTest.h +++ b/Framework/Algorithms/test/AppendSpectraTest.h @@ -135,7 +135,7 @@ class AppendSpectraTest : public CxxTest::TestSuite { void test_event_workspaces_with_multiple_appends() { doTypeAndParamsTest(true, false, 3); } - void test_event_workspaces_with_merging_logs_and_multiple_appends() { (true, true, 3); } + void test_event_workspaces_with_merging_logs_and_multiple_appends() { doTypeAndParamsTest(true, true, 3); } void test_2D_workspaces() { doTypeAndParamsTest(false); } @@ -409,11 +409,11 @@ class AppendSpectraTest : public CxxTest::TestSuite { size_t i, j; for (i = 0; i < ws1num; ++i) { - ws1Values[i] = i; + ws1Values[i] = static_cast(i); } for (j = i; j < (ws1num + ws2num); j++) { - ws2Values[j - ws1num] = j - (isBinEdges ? 1 : 0); + ws2Values[j - ws1num] = static_cast(j - (isBinEdges ? 1 : 0)); } std::unique_ptr ws1Axis; diff --git a/docs/source/release/v6.9.0/Framework/Algorithms/New_features/36345.rst b/docs/source/release/v6.9.0/Framework/Algorithms/New_features/36345.rst index 36917f055ef6..7c4308790df4 100644 --- a/docs/source/release/v6.9.0/Framework/Algorithms/New_features/36345.rst +++ b/docs/source/release/v6.9.0/Framework/Algorithms/New_features/36345.rst @@ -1 +1 @@ -- Add a new option (AppendYAxisLabels) to :ref:`AppendSpectra ` to append y-axis values from input workspaces to the input workspace \ No newline at end of file +- Add a new option (AppendYAxisLabels) to :ref:`AppendSpectra ` to append y-axis values from input workspaces to the output workspace \ No newline at end of file From 8d357742811990ebb109ac547eef07c3f722ae19 Mon Sep 17 00:00:00 2001 From: Mohammed Almakki <44416400+MohamedAlmaki@users.noreply.github.com> Date: Mon, 13 Nov 2023 15:51:29 +0000 Subject: [PATCH 09/12] Delete wrong checked file --- .../kernel/src/Exports/LambdaValidator.cpp | 21 ------------------- 1 file changed, 21 deletions(-) delete mode 100644 Framework/PythonInterface/mantid/kernel/src/Exports/LambdaValidator.cpp diff --git a/Framework/PythonInterface/mantid/kernel/src/Exports/LambdaValidator.cpp b/Framework/PythonInterface/mantid/kernel/src/Exports/LambdaValidator.cpp deleted file mode 100644 index 0696ad8bc5ce..000000000000 --- a/Framework/PythonInterface/mantid/kernel/src/Exports/LambdaValidator.cpp +++ /dev/null @@ -1,21 +0,0 @@ -// Mantid Repository : https://github.com/mantidproject/mantid -// -// Copyright © 2023 ISIS Rutherford Appleton Laboratory UKRI, -// NScD Oak Ridge National Laboratory, European Spallation Source, -// Institut Laue - Langevin & CSNS, Institute of High Energy Physics, CAS -// SPDX - License - Identifier: GPL - 3.0 + -#include "MantidKernel/LambdaValidator.h" -#include "MantidPythonInterface/core/GetPointer.h" -#include -#include - -using Mantid::Kernel::LambdaValidator; -using namespace boost::python; - -GET_POINTER_SPECIALIZATION(LambdaValidator) - -void export_LambdaValidator() { - register_ptr_to_python>(); - - class_("IValidator", no_init); -} From 5a8024c6c16b6cc1f60eeaabe35870dc6a371f56 Mon Sep 17 00:00:00 2001 From: Mohammed Almakki Date: Mon, 27 Nov 2023 18:26:12 +0000 Subject: [PATCH 10/12] Add error message for axis with type mismatch --- Framework/Algorithms/src/AppendSpectra.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/Framework/Algorithms/src/AppendSpectra.cpp b/Framework/Algorithms/src/AppendSpectra.cpp index d8ef49d85ee1..dfb7c7464482 100644 --- a/Framework/Algorithms/src/AppendSpectra.cpp +++ b/Framework/Algorithms/src/AppendSpectra.cpp @@ -146,8 +146,18 @@ void AppendSpectra::appendYAxisLabels(const MatrixWorkspace &ws1, const MatrixWo const auto outputYAxis = output.getAxis(yAxisNum); const auto ws1len = ws1.getNumberHistograms(); + const bool isSpectra = yAxisWS1->isSpectra() && yAxisWS2->isSpectra(); const bool isTextAxis = yAxisWS1->isText() && yAxisWS2->isText(); const bool isNumericAxis = yAxisWS1->isNumeric() && yAxisWS2->isNumeric(); + + if (!isSpectra && !isTextAxis && !isNumericAxis) { + const std::string message( + "Y-Axis type mistmach. Ensure that the Y-axis types in both workspaces match. The Y-axis should be set to the " + "same type in each workspace, whether it be Numeric, Spectra, or Text."); + + throw std::invalid_argument(message); + } + bool isBinEdgeAxis = dynamic_cast(yAxisWS1) != nullptr && dynamic_cast(yAxisWS2) != nullptr; auto outputTextAxis = dynamic_cast(outputYAxis); From dd4898e319c24e4eedff8d2bf90e5ad528452244 Mon Sep 17 00:00:00 2001 From: Mohammed Almakki Date: Mon, 27 Nov 2023 20:50:01 +0000 Subject: [PATCH 11/12] Add unit test for the error message and update docs --- Framework/Algorithms/test/AppendSpectraTest.h | 66 +++++++++++-------- docs/source/algorithms/AppendSpectra-v1.rst | 3 +- 2 files changed, 41 insertions(+), 28 deletions(-) diff --git a/Framework/Algorithms/test/AppendSpectraTest.h b/Framework/Algorithms/test/AppendSpectraTest.h index 47bbc0831bbf..c7cb9fccb19c 100644 --- a/Framework/Algorithms/test/AppendSpectraTest.h +++ b/Framework/Algorithms/test/AppendSpectraTest.h @@ -38,7 +38,7 @@ class AppendSpectraTest : public CxxTest::TestSuite { void test_algorithm_execution() { std::string top = "top"; std::string bottom = "bottom"; - setupRealWorkspaces(top, bottom); + createNonOverlappingWorkspaces(top, bottom); // Get the two input workspaces for later MatrixWorkspace_sptr in1 = AnalysisDataService::Instance().retrieveWS(top); @@ -76,7 +76,7 @@ class AppendSpectraTest : public CxxTest::TestSuite { void test_algorithm_execution_with_multiple_appends() { std::string top = "top"; std::string bottom = "bottom"; - setupRealWorkspaces(top, bottom); + createNonOverlappingWorkspaces(top, bottom); // Get the two input workspaces for later MatrixWorkspace_sptr in1 = AnalysisDataService::Instance().retrieveWS(top); @@ -84,7 +84,7 @@ class AppendSpectraTest : public CxxTest::TestSuite { // Now it should succeed auto appendSpectra = Mantid::API::AlgorithmManager::Instance().create("AppendSpectra"); - doAppendSpectraWithWorkspacesTest(appendSpectra, top, bottom, top, false, false, 2); + doAppendSpectraWithWorkspacesTest(appendSpectra, top, bottom, top, false, false, false, 2); MatrixWorkspace_const_sptr output; TS_ASSERT_THROWS_NOTHING(output = AnalysisDataService::Instance().retrieveWS(top)); @@ -293,6 +293,10 @@ class AppendSpectraTest : public CxxTest::TestSuite { void test_append_y_axis_with_bin_edges_axis_disabled() { doAppendWorkspacesWithNumericAxisTest(false, true, false); } + void test_append_y_axis_with_different_y_axis_type() { + doAppendWorkspacesWithNumericAxisTest(false, false, false, true); + } + private: void doTypeAndParamsTest(bool event, bool combineLogs = false, int number = 1) { const std::string ws1Name = "AppendSpectraTest_grp1"; @@ -324,7 +328,7 @@ class AppendSpectraTest : public CxxTest::TestSuite { AnalysisDataService::Instance().addOrReplace(ws2Name, ws2); auto appendSpectra = Mantid::API::AlgorithmManager::Instance().create("AppendSpectra"); - doAppendSpectraWithWorkspacesTest(appendSpectra, ws1Name, ws2Name, ws1Name, false, combineLogs, number); + doAppendSpectraWithWorkspacesTest(appendSpectra, ws1Name, ws2Name, ws1Name, false, false, combineLogs, number); TS_ASSERT_THROWS_NOTHING( out = std::dynamic_pointer_cast(AnalysisDataService::Instance().retrieve(ws1Name));) @@ -352,8 +356,8 @@ class AppendSpectraTest : public CxxTest::TestSuite { void doAppendSpectraWithWorkspacesTest(IAlgorithm_sptr appendSpectra, const std::string &inputWorkspace1, const std::string &inputWorkspace2, const std::string &outputWorkspace, - const bool appendYAxis = false, const bool combineLogs = false, - const int number = 1) { + const bool shoudThrow = false, const bool appendYAxis = false, + const bool combineLogs = false, const int number = 1) { if (!appendSpectra->isInitialized()) appendSpectra->initialize(); TS_ASSERT_THROWS_NOTHING(appendSpectra->setRethrows(true)); @@ -363,39 +367,47 @@ class AppendSpectraTest : public CxxTest::TestSuite { TS_ASSERT_THROWS_NOTHING(appendSpectra->setProperty("Number", number)); TS_ASSERT_THROWS_NOTHING(appendSpectra->setProperty("MergeLogs", combineLogs)); TS_ASSERT_THROWS_NOTHING(appendSpectra->setProperty("OutputWorkspace", outputWorkspace)); - TS_ASSERT_THROWS_NOTHING(appendSpectra->execute()); - TS_ASSERT(appendSpectra->isExecuted()); + + if (shoudThrow) { + TS_ASSERT_THROWS_ANYTHING(appendSpectra->execute()); + } else { + TS_ASSERT_THROWS_NOTHING(appendSpectra->execute()); + TS_ASSERT(appendSpectra->isExecuted()); + } } - void doAppendWorkspacesWithNumericAxisTest(bool appendYAxis, bool isBinEdgeAxis, bool overlapped) { - std::string inputWorkspace2; - std::string inputWorkspace1; + void doAppendWorkspacesWithNumericAxisTest(bool appendYAxis, bool isBinEdgeAxis, bool overlappedWS, + bool differentTypes = false) { + const std::string inputWorkspace1 = "top"; + const std::string inputWorkspace2 = "bottom"; std::string outputWorkspace = "appended"; - if (overlapped) { - inputWorkspace1 = "weRebinned1"; - inputWorkspace2 = "weRebinned2"; + if (overlappedWS || differentTypes) { createWorkspaceWithAxisAndLabel(inputWorkspace1, "Time", "1.0"); - createWorkspaceWithAxisAndLabel(inputWorkspace2, "Time", "1.0"); + createWorkspaceWithAxisAndLabel(inputWorkspace2, differentTypes ? "Text" : "Time", "1.0"); } else { - inputWorkspace1 = "top"; - inputWorkspace2 = "bottom"; - setupRealWorkspaces(inputWorkspace1, inputWorkspace2); + createNonOverlappingWorkspaces(inputWorkspace1, inputWorkspace2); } MatrixWorkspace_sptr inputWS1 = AnalysisDataService::Instance().retrieveWS(inputWorkspace1); MatrixWorkspace_sptr inputWS2 = AnalysisDataService::Instance().retrieveWS(inputWorkspace2); - appendNumericAxis(inputWS1, inputWS2, isBinEdgeAxis); + if (!differentTypes) { + appendNumericAxis(inputWS1, inputWS2, isBinEdgeAxis); + } auto appendSpectra = Mantid::API::AlgorithmManager::Instance().create("AppendSpectra"); - doAppendSpectraWithWorkspacesTest(appendSpectra, inputWorkspace1, inputWorkspace2, outputWorkspace, appendYAxis); - - bool isNotOverlappedAndDisabledAppend = !appendYAxis && !overlapped; - MatrixWorkspace_const_sptr outputWS = AnalysisDataService::Instance().retrieveWS(outputWorkspace); - auto outputAxis = outputWS->getAxis(1); - for (size_t i = 1; i < outputWS->getNumberHistograms(); ++i) { - TS_ASSERT_EQUALS(outputAxis->getValue(i), isNotOverlappedAndDisabledAppend ? 0 : i); + doAppendSpectraWithWorkspacesTest(appendSpectra, inputWorkspace1, inputWorkspace2, outputWorkspace, differentTypes, + appendYAxis); + + if (!differentTypes) { + bool isNotOverlappedAndDisabledAppend = !appendYAxis && !overlappedWS; + MatrixWorkspace_const_sptr outputWS = + AnalysisDataService::Instance().retrieveWS(outputWorkspace); + auto outputAxis = outputWS->getAxis(1); + for (size_t i = 1; i < outputWS->getNumberHistograms(); ++i) { + TS_ASSERT_EQUALS(outputAxis->getValue(i), isNotOverlappedAndDisabledAppend ? 0 : i); + } } } @@ -469,7 +481,7 @@ class AppendSpectraTest : public CxxTest::TestSuite { TS_ASSERT(rebin->isExecuted()); } - void setupRealWorkspaces(std::string ws1, std::string ws2) { + void createNonOverlappingWorkspaces(std::string ws1, std::string ws2) { IAlgorithm *loader; loader = new Mantid::DataHandling::LoadRaw3; loader->initialize(); diff --git a/docs/source/algorithms/AppendSpectra-v1.rst b/docs/source/algorithms/AppendSpectra-v1.rst index 7ff749361eb1..630eb9909f92 100644 --- a/docs/source/algorithms/AppendSpectra-v1.rst +++ b/docs/source/algorithms/AppendSpectra-v1.rst @@ -46,7 +46,8 @@ Note that when spectra numbers do not overlap, it doesn't automatically imply that y-axis values are carried over from previous workspaces. To address this, use the 'AppendYAxisLabels' option. This will combine y-axis values from two input workspaces into the new output workspace, -arranging them in the order of the first workspace followed by the second. +arranging them in the order of the first workspace followed by the second. In addition, the axes +should have the same type. .. seealso:: :ref:`algm-ConjoinWorkspaces` for joining parts of the same workspace. From ea55c68b31271ea141091719f95f28dc2fe8b8f7 Mon Sep 17 00:00:00 2001 From: Mohammed Almakki Date: Thu, 30 Nov 2023 11:29:26 +0000 Subject: [PATCH 12/12] Fix a few typos --- Framework/Algorithms/src/AppendSpectra.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Framework/Algorithms/src/AppendSpectra.cpp b/Framework/Algorithms/src/AppendSpectra.cpp index dfb7c7464482..fe8e46f0dbd2 100644 --- a/Framework/Algorithms/src/AppendSpectra.cpp +++ b/Framework/Algorithms/src/AppendSpectra.cpp @@ -144,7 +144,7 @@ void AppendSpectra::appendYAxisLabels(const MatrixWorkspace &ws1, const MatrixWo const auto yAxisWS1 = ws1.getAxis(yAxisNum); const auto yAxisWS2 = ws2.getAxis(yAxisNum); const auto outputYAxis = output.getAxis(yAxisNum); - const auto ws1len = ws1.getNumberHistograms(); + const auto ws1Len = ws1.getNumberHistograms(); const bool isSpectra = yAxisWS1->isSpectra() && yAxisWS2->isSpectra(); const bool isTextAxis = yAxisWS1->isText() && yAxisWS2->isText(); @@ -152,7 +152,7 @@ void AppendSpectra::appendYAxisLabels(const MatrixWorkspace &ws1, const MatrixWo if (!isSpectra && !isTextAxis && !isNumericAxis) { const std::string message( - "Y-Axis type mistmach. Ensure that the Y-axis types in both workspaces match. The Y-axis should be set to the " + "Y-Axis type mismatch. Ensure that the Y-axis types in both workspaces match. The Y-axis should be set to the " "same type in each workspace, whether it be Numeric, Spectra, or Text."); throw std::invalid_argument(message); @@ -161,16 +161,16 @@ void AppendSpectra::appendYAxisLabels(const MatrixWorkspace &ws1, const MatrixWo bool isBinEdgeAxis = dynamic_cast(yAxisWS1) != nullptr && dynamic_cast(yAxisWS2) != nullptr; auto outputTextAxis = dynamic_cast(outputYAxis); - const auto ouputlen = output.getNumberHistograms() + (isBinEdgeAxis ? 1 : 0); - for (size_t i = 0; i < ouputlen; ++i) { + const auto outputLen = output.getNumberHistograms() + (isBinEdgeAxis ? 1 : 0); + for (size_t i = 0; i < outputLen; ++i) { if (isTextAxis) { // check if we're outside the spectra of the first workspace - const std::string inputLabel = i < ws1len ? yAxisWS1->label(i) : yAxisWS2->label(i - ws1len); + const std::string inputLabel = i < ws1Len ? yAxisWS1->label(i) : yAxisWS2->label(i - ws1Len); outputTextAxis->setLabel(i, !inputLabel.empty() ? inputLabel : ""); } else if (isNumericAxis) { // check if we're outside the spectra of the first workspace - const double inputVal = i < ws1len ? yAxisWS1->getValue(i) : yAxisWS2->getValue(i - ws1len); + const double inputVal = i < ws1Len ? yAxisWS1->getValue(i) : yAxisWS2->getValue(i - ws1Len); outputYAxis->setValue(i, inputVal); } }