Merge pull request #38619 from mantidproject/fix_conjoin_workspaces_bug

Fix logic for checking bin consistency in ConjoinWorkspaces algorithm
mantidproject · Jan 20, 2025 · 48117b6 · 48117b6
2 parents 1541ebc + c155bd1
commit 48117b6
Show file tree

Hide file tree

Showing 5 changed files with 44 additions and 10 deletions.
diff --git 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/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
@@ -56,14 +56,15 @@ 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);
   }
-  // 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

diff --git 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

diff --git 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;
@@ -412,7 +428,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();