From 517eecff220802fc649b5aa9387216e14210a56c Mon Sep 17 00:00:00 2001 From: Tom Hampson Date: Fri, 10 Jan 2025 12:18:42 +0000 Subject: [PATCH 1/5] Remove old commend It doesn't make any sense and just confused me --- Framework/Algorithms/test/ConjoinWorkspacesTest.h | 1 - 1 file changed, 1 deletion(-) diff --git a/Framework/Algorithms/test/ConjoinWorkspacesTest.h b/Framework/Algorithms/test/ConjoinWorkspacesTest.h index 01167fc220d6..6bb5b43205bd 100644 --- a/Framework/Algorithms/test/ConjoinWorkspacesTest.h +++ b/Framework/Algorithms/test/ConjoinWorkspacesTest.h @@ -412,7 +412,6 @@ class ConjoinWorkspacesTest : public CxxTest::TestSuite { MatrixWorkspace_sptr ews = WorkspaceCreationHelper::createEventWorkspace(10, 10); AnalysisDataService::Instance().addOrReplace(name, ews); - // Crop ews to have first 3 spectra, ews2 to have second 3 CropWorkspace crop; crop.setChild(true); crop.initialize(); From 7e1b17f89e491c1c0a9d7934cff1be4e159ef51a Mon Sep 17 00:00:00 2001 From: Tom Hampson Date: Fri, 10 Jan 2025 14:41:31 +0000 Subject: [PATCH 2/5] Update misleading function documentation. The function does not compare sizes of workspaces, so the comment was wrong. --- Framework/Algorithms/src/WorkspaceJoiners.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Framework/Algorithms/src/WorkspaceJoiners.cpp b/Framework/Algorithms/src/WorkspaceJoiners.cpp index 51927f043461..8b5dd5e26612 100644 --- a/Framework/Algorithms/src/WorkspaceJoiners.cpp +++ b/Framework/Algorithms/src/WorkspaceJoiners.cpp @@ -148,8 +148,7 @@ DataObjects::EventWorkspace_sptr WorkspaceJoiners::execEvent(const DataObjects:: return output; } -/** Checks that the two input workspace have common size and the same - * instrument & unit. +/** Checks that the two input workspace have the same instrument, unit and distribution flag. * @param ws1 :: The first input workspace * @param ws2 :: The second input workspace * @throw std::invalid_argument If the workspaces are not compatible From b555a4f4b4033ff2cf611c5b3ab064accf667b6d Mon Sep 17 00:00:00 2001 From: Tom Hampson Date: Fri, 10 Jan 2025 17:19:47 +0000 Subject: [PATCH 3/5] Add ConjoinWorkspaces test to ensure it works for matching bins. --- .../Algorithms/test/ConjoinWorkspacesTest.h | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/Framework/Algorithms/test/ConjoinWorkspacesTest.h b/Framework/Algorithms/test/ConjoinWorkspacesTest.h index 6bb5b43205bd..96e93573c0a1 100644 --- a/Framework/Algorithms/test/ConjoinWorkspacesTest.h +++ b/Framework/Algorithms/test/ConjoinWorkspacesTest.h @@ -169,7 +169,7 @@ class ConjoinWorkspacesTest : public CxxTest::TestSuite { TS_ASSERT(!conj.isExecuted()); } - void testCheckMatchingBinsError() { + void testNonMatchingBinsThrowsError() { MatrixWorkspace_sptr ws1, ws2; ws1 = WorkspaceCreationHelper::createEventWorkspace(10, 5); ws2 = WorkspaceCreationHelper::createEventWorkspace(10, 10); @@ -192,6 +192,22 @@ class ConjoinWorkspacesTest : public CxxTest::TestSuite { } } + void testMatchingBinsThrowsNoError() { + MatrixWorkspace_sptr ws1 = WorkspaceCreationHelper::createEventWorkspace(10, 5); + MatrixWorkspace_sptr ws2 = WorkspaceCreationHelper::createEventWorkspace(10, 5); + + // CheckMatchingBins is true by default, so don't set it here. + ConjoinWorkspaces conj; + conj.initialize(); + conj.setProperty("CheckOverlapping", false); + conj.setProperty("InputWorkspace1", ws1); + conj.setProperty("InputWorkspace2", ws2); + conj.setAlwaysStoreInADS(false); + conj.setRethrows(true); + + TS_ASSERT_THROWS_NOTHING(conj.execute()); + } + void testDoCheckForOverlap() { MatrixWorkspace_sptr ws1, ws2; int numPixels = 10; From e63d5c8e507d63bef65d27b6f76623fc57aa5269 Mon Sep 17 00:00:00 2001 From: Tom Hampson Date: Fri, 17 Jan 2025 11:45:05 +0000 Subject: [PATCH 4/5] Grammatical fix for warning messages --- Framework/Algorithms/src/AppendSpectra.cpp | 4 ++-- Framework/Algorithms/src/ConjoinWorkspaces.cpp | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Framework/Algorithms/src/AppendSpectra.cpp b/Framework/Algorithms/src/AppendSpectra.cpp index fe8e46f0dbd2..4d687f5e8785 100644 --- a/Framework/Algorithms/src/AppendSpectra.cpp +++ b/Framework/Algorithms/src/AppendSpectra.cpp @@ -68,8 +68,8 @@ void AppendSpectra::exec() { if (((eventWs1) && (!eventWs2)) || ((!eventWs1) && (eventWs2))) { const std::string message("Only one of the input workspaces are of type " "EventWorkspace; please use matching workspace " - "types (both EventWorkspace's or both " - "Workspace2D's)."); + "types (both EventWorkspace or both " + "Workspace2D)."); g_log.error(message); throw std::invalid_argument(message); } diff --git a/Framework/Algorithms/src/ConjoinWorkspaces.cpp b/Framework/Algorithms/src/ConjoinWorkspaces.cpp index 1ce2e65e6031..bac02eeeb34c 100644 --- a/Framework/Algorithms/src/ConjoinWorkspaces.cpp +++ b/Framework/Algorithms/src/ConjoinWorkspaces.cpp @@ -56,8 +56,8 @@ void ConjoinWorkspaces::exec() { if (((eventWs1) && (!eventWs2)) || ((!eventWs1) && (eventWs2))) { const std::string message("Only one of the input workspaces are of type " "EventWorkspace; please use matching workspace " - "types (both EventWorkspace's or both " - "Workspace2D's)."); + "types (both EventWorkspace or both " + "Workspace2D)."); g_log.error(message); throw std::invalid_argument(message); } From c155bd1b52af32a3ebcf695fbcbffc2d54c85ff5 Mon Sep 17 00:00:00 2001 From: Tom Hampson Date: Fri, 17 Jan 2025 11:50:11 +0000 Subject: [PATCH 5/5] Fix logic for checking bin consistency in ConjoinWorkspaces algorithm --- .../inc/MantidAlgorithms/ConjoinWorkspaces.h | 1 + .../Algorithms/src/ConjoinWorkspaces.cpp | 23 +++++++++++++++++-- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/Framework/Algorithms/inc/MantidAlgorithms/ConjoinWorkspaces.h b/Framework/Algorithms/inc/MantidAlgorithms/ConjoinWorkspaces.h index ecbbda41eac7..68a320df60e4 100644 --- a/Framework/Algorithms/inc/MantidAlgorithms/ConjoinWorkspaces.h +++ b/Framework/Algorithms/inc/MantidAlgorithms/ConjoinWorkspaces.h @@ -55,6 +55,7 @@ class MANTID_ALGORITHMS_DLL ConjoinWorkspaces : public WorkspaceJoiners { void init() override; void exec() override; + bool checkBinning(const API::MatrixWorkspace_const_sptr &ws1, const API::MatrixWorkspace_const_sptr &ws2) const; void checkForOverlap(const API::MatrixWorkspace &ws1, const API::MatrixWorkspace &ws2, bool checkSpectra) const; API::MatrixWorkspace_sptr conjoinEvents(const DataObjects::EventWorkspace &ws1, const DataObjects::EventWorkspace &ws2); diff --git a/Framework/Algorithms/src/ConjoinWorkspaces.cpp b/Framework/Algorithms/src/ConjoinWorkspaces.cpp index bac02eeeb34c..e55ecbcd8d9b 100644 --- a/Framework/Algorithms/src/ConjoinWorkspaces.cpp +++ b/Framework/Algorithms/src/ConjoinWorkspaces.cpp @@ -61,9 +61,10 @@ void ConjoinWorkspaces::exec() { g_log.error(message); throw std::invalid_argument(message); } - // Check if bins match + + // Check whether bins match bool checkBins = getProperty("CheckMatchingBins"); - if (!WorkspaceHelpers::matchingBins(ws1, ws2) && checkBins) { + if (checkBins && !checkBinning(ws1, ws2)) { const std::string message("The bins do not match in the input workspaces. " "Consider using RebinToWorkspace to preprocess " "the workspaces before conjoining them."); @@ -88,6 +89,24 @@ void ConjoinWorkspaces::exec() { AnalysisDataService::Instance().remove(getPropertyValue("InputWorkspace2")); } +//---------------------------------------------------------------------------------------------- +/** Checks whether the binning is consistent between two workspaces + * @param ws1 :: The first input workspace + * @param ws2 :: The second input workspace + * @return :: true if both workspaces have consistent binning. + */ +bool ConjoinWorkspaces::checkBinning(const API::MatrixWorkspace_const_sptr &ws1, + const API::MatrixWorkspace_const_sptr &ws2) const { + if (ws1->isRaggedWorkspace() || ws2->isRaggedWorkspace()) { + return false; + } + + // If neither workspace is ragged, we only need to check the first specrum. + // Otherwise the matchingBins() function requires the two workspaces to have + // the same number of spectra. + return WorkspaceHelpers::matchingBins(ws1, ws2, true); +} + //---------------------------------------------------------------------------------------------- /** Checks that the two input workspaces have non-overlapping spectra numbers * and contributing detectors