Merge pull request #38685 from mantidproject/fix_conjoin_workspaces_b…

…ug_ornl_next

Fix conjoin workspaces bug ornl next

Framework/API/inc/MantidAPI/WorkspaceOpOverloads.h

@@ -54,9 +54,10 @@ MatrixWorkspace_sptr MANTID_API_DLL operator/=(const MatrixWorkspace_sptr &lhs,
 */
 struct MANTID_API_DLL WorkspaceHelpers {
   // Checks whether the binning is the same in two workspaces
-  static bool matchingBins(const MatrixWorkspace &ws1, const MatrixWorkspace &ws2, const bool firstOnly = false);
+  static bool matchingBins(const std::shared_ptr<const MatrixWorkspace> &ws1,
+                           const std::shared_ptr<const MatrixWorkspace> &ws2, const bool firstOnly = false);
   // Checks whether a the X vectors in a workspace are actually the same vector
-  static bool sharedXData(const MatrixWorkspace &WS);
+  static bool sharedXData(const std::shared_ptr<const MatrixWorkspace> &WS);
   // Divides the data in a workspace by the bin width to make it a distribution
   // (or the reverse)
   static void makeDistribution(const MatrixWorkspace_sptr &workspace, const bool forwards = true);

Framework/API/src/WorkspaceOpOverloads.cpp

@@ -367,17 +367,18 @@ MatrixWorkspace_sptr operator/=(const MatrixWorkspace_sptr &lhs, const double &r
  * number of spectra
  *  @return True if the test passes
  */
-bool WorkspaceHelpers::matchingBins(const MatrixWorkspace &ws1, const MatrixWorkspace &ws2, const bool firstOnly) {
+bool WorkspaceHelpers::matchingBins(const std::shared_ptr<const MatrixWorkspace> &ws1,
+                                    const std::shared_ptr<const MatrixWorkspace> &ws2, const bool firstOnly) {
   // First of all, the first vector must be the same size
-  if (ws1.x(0).size() != ws2.x(0).size())
+  if (ws1->x(0).size() != ws2->x(0).size())
     return false;
 
   // Now check the first spectrum
-  const double firstWS = std::accumulate(ws1.x(0).begin(), ws1.x(0).end(), 0.);
-  const double secondWS = std::accumulate(ws2.x(0).begin(), ws2.x(0).end(), 0.);
+  const double firstWS = std::accumulate(ws1->x(0).begin(), ws1->x(0).end(), 0.);
+  const double secondWS = std::accumulate(ws2->x(0).begin(), ws2->x(0).end(), 0.);
   if (std::abs(firstWS) < 1.0E-7 && std::abs(secondWS) < 1.0E-7) {
-    for (size_t i = 0; i < ws1.x(0).size(); i++) {
-      if (!Kernel::withinAbsoluteDifference(ws1.x(0)[i], ws2.x(0)[i], 1.0E-7))
+    for (size_t i = 0; i < ws1->x(0).size(); i++) {
+      if (!Kernel::withinAbsoluteDifference(ws1->x(0)[i], ws2->x(0)[i], 1.0E-7))
         return false;
     }
   } else if (!Kernel::withinRelativeDifference(firstWS, secondWS, 1.0E-7))
@@ -388,15 +389,15 @@ bool WorkspaceHelpers::matchingBins(const MatrixWorkspace &ws1, const MatrixWork
     return true;
 
   // Check that total size of workspace is the same
-  if (ws1.size() != ws2.size())
+  if (ws1->size() != ws2->size())
     return false;
   // If that passes then check whether all the X vectors are shared
   if (sharedXData(ws1) && sharedXData(ws2))
     return true;
 
   // If that didn't pass then explicitly check 1 in 10 of the vectors (min 10,
   // max 100)
-  const size_t numHist = ws1.getNumberHistograms();
+  const size_t numHist = ws1->getNumberHistograms();
   size_t numberToCheck = numHist / 10;
   if (numberToCheck < 10)
     numberToCheck = 10;
@@ -406,11 +407,11 @@ bool WorkspaceHelpers::matchingBins(const MatrixWorkspace &ws1, const MatrixWork
   if (!step)
     step = 1;
   for (size_t i = step; i < numHist; i += step) {
-    const double firstWSLoop = std::accumulate(ws1.x(i).begin(), ws1.x(i).end(), 0.);
-    const double secondWSLoop = std::accumulate(ws2.x(i).begin(), ws2.x(i).end(), 0.);
+    const double firstWSLoop = std::accumulate(ws1->x(i).begin(), ws1->x(i).end(), 0.);
+    const double secondWSLoop = std::accumulate(ws2->x(i).begin(), ws2->x(i).end(), 0.);
     if (std::abs(firstWSLoop) < 1.0E-7 && std::abs(secondWSLoop) < 1.0E-7) {
-      for (size_t j = 0; j < ws1.x(i).size(); j++) {
-        if (!Kernel::withinAbsoluteDifference(ws1.x(i)[j], ws2.x(i)[j], 1.0E-7))
+      for (size_t j = 0; j < ws1->x(i).size(); j++) {
+        if (!Kernel::withinAbsoluteDifference(ws1->x(i)[j], ws2->x(i)[j], 1.0E-7))
           return false;
       }
     } else if (!Kernel::withinRelativeDifference(firstWSLoop, secondWSLoop, 1.0E-7))
@@ -421,11 +422,11 @@ bool WorkspaceHelpers::matchingBins(const MatrixWorkspace &ws1, const MatrixWork
 }
 
 /// Checks whether all the X vectors in a workspace are the same one underneath
-bool WorkspaceHelpers::sharedXData(const MatrixWorkspace &WS) {
-  const double &first = WS.x(0)[0];
-  const size_t numHist = WS.getNumberHistograms();
+bool WorkspaceHelpers::sharedXData(const std::shared_ptr<const MatrixWorkspace> &WS) {
+  const double &first = WS->x(0)[0];
+  const size_t numHist = WS->getNumberHistograms();
   for (size_t i = 1; i < numHist; ++i) {
-    if (&first != &(WS.x(i)[0]))
+    if (&first != &(WS->x(i)[0]))
       return false;
   }
   return true;

Framework/API/test/WorkspaceOpOverloadsTest.h

@@ -23,20 +23,20 @@ class WorkspaceOpOverloadsTest : public CxxTest::TestSuite {
   void test_matchingBins() {
     auto ws = std::make_shared<WorkspaceTester>();
     ws->initialize(2, 2, 1);
-    TSM_ASSERT("Passing it the same workspace twice had better work!", WorkspaceHelpers::matchingBins(*ws, *ws));
+    TSM_ASSERT("Passing it the same workspace twice had better work!", WorkspaceHelpers::matchingBins(ws, ws));
 
     // Different size workspaces fail of course
     auto ws2 = std::make_shared<WorkspaceTester>();
     ws2->initialize(3, 2, 1);
     auto ws3 = std::make_shared<WorkspaceTester>();
     ws3->initialize(2, 3, 2);
-    TSM_ASSERT("Different size workspaces should always fail", !WorkspaceHelpers::matchingBins(*ws, *ws2));
-    TSM_ASSERT("Different size workspaces should always fail", !WorkspaceHelpers::matchingBins(*ws, *ws3));
+    TSM_ASSERT("Different size workspaces should always fail", !WorkspaceHelpers::matchingBins(ws, ws2));
+    TSM_ASSERT("Different size workspaces should always fail", !WorkspaceHelpers::matchingBins(ws, ws3));
 
     ws2->dataX(1)[0] = 99.0;
     TSM_ASSERT("First-spectrum-only check should pass even when things differ "
                "in later spectra",
-               WorkspaceHelpers::matchingBins(*ws, *ws2, true));
+               WorkspaceHelpers::matchingBins(ws, ws2, true));
 
     // Check it fails if the sum is zero but the boundaries differ, both for 1st
     // & later spectra.
@@ -48,7 +48,7 @@ class WorkspaceOpOverloadsTest : public CxxTest::TestSuite {
     ws5->initialize(2, 3, 2);
     ws5->dataX(0)[0] = -1;
     ws5->dataX(0)[2] = 0;
-    TS_ASSERT(!WorkspaceHelpers::matchingBins(*ws4, *ws5, true))
+    TS_ASSERT(!WorkspaceHelpers::matchingBins(ws4, ws5, true))
     auto ws6 = std::make_shared<WorkspaceTester>();
     ws6->initialize(2, 3, 2);
     ws6->dataX(1)[0] = -1;
@@ -57,7 +57,7 @@ class WorkspaceOpOverloadsTest : public CxxTest::TestSuite {
     ws7->initialize(2, 3, 2);
     ws7->dataX(1)[0] = -1;
     ws7->dataX(1)[2] = 0;
-    TS_ASSERT(!WorkspaceHelpers::matchingBins(*ws6, *ws7))
+    TS_ASSERT(!WorkspaceHelpers::matchingBins(ws6, ws7))
 
     // N.B. There are known ways to fool this method, but they are considered
     // acceptable because
@@ -81,25 +81,25 @@ class WorkspaceOpOverloadsTest : public CxxTest::TestSuite {
     ws2->getSpectrum(1).dataX()[0] = -2.7;
     ws2->getSpectrum(1).dataX()[1] = -1.7;
 
-    TS_ASSERT(WorkspaceHelpers::matchingBins(*ws1, *ws2, true));
-    TS_ASSERT(!WorkspaceHelpers::matchingBins(*ws1, *ws2));
+    TS_ASSERT(WorkspaceHelpers::matchingBins(ws1, ws2, true));
+    TS_ASSERT(!WorkspaceHelpers::matchingBins(ws1, ws2));
 
     ws1->getSpectrum(0).dataX()[0] = -2.0;
     ws1->getSpectrum(0).dataX()[1] = -1.0;
     ws2->getSpectrum(0).dataX()[0] = -3.0;
     ws2->getSpectrum(0).dataX()[1] = -4.0;
 
-    TS_ASSERT(!WorkspaceHelpers::matchingBins(*ws1, *ws2, true));
+    TS_ASSERT(!WorkspaceHelpers::matchingBins(ws1, ws2, true));
   }
 
   void test_sharedXData() {
     auto ws = std::make_shared<WorkspaceTester>();
     ws->initialize(2, 2, 1);
     // By default the X vectors are different ones
-    TS_ASSERT(!WorkspaceHelpers::sharedXData(*ws));
+    TS_ASSERT(!WorkspaceHelpers::sharedXData(ws));
     // Force both X spectra to point to the same underlying vector
     ws->getSpectrum(1).setX(ws->getSpectrum(0).ptrX());
-    TS_ASSERT(WorkspaceHelpers::sharedXData(*ws));
+    TS_ASSERT(WorkspaceHelpers::sharedXData(ws));
   }
 
   void test_makeDistribution() {

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);

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);
   }

Framework/Algorithms/src/BinaryOperation.cpp

@@ -385,7 +385,7 @@ std::string BinaryOperation::checkSizeCompatibility(const API::MatrixWorkspace_c
     return "";
   // Past this point, we require the X arrays to match. Note this only checks
   // the first spectrum except for ragged workspaces
-  if (!WorkspaceHelpers::matchingBins(*lhs, *rhs, !m_lhsRagged && !m_rhsRagged)) {
+  if (!WorkspaceHelpers::matchingBins(lhs, rhs, !m_lhsRagged && !m_rhsRagged)) {
     return "X arrays must match when performing this operation on a 2D "
            "workspaces.";
   }

Framework/Algorithms/src/CalculateTransmission.cpp

@@ -130,7 +130,7 @@ void CalculateTransmission::exec() {
   const bool usingSameInstrument = sampleWS->getInstrument()->getName() == directWS->getInstrument()->getName();
   if (!usingSameInstrument)
     throw std::invalid_argument("The input workspaces do not come from the same instrument.");
-  if (!WorkspaceHelpers::matchingBins(*sampleWS, *directWS))
+  if (!WorkspaceHelpers::matchingBins(sampleWS, directWS))
     throw std::invalid_argument("The input workspaces do not have matching bins.");
 
   bool usingMonitor = !isEmpty(transMonitorID);

Framework/Algorithms/src/CalculateTransmissionBeamSpreader.cpp

@@ -90,9 +90,9 @@ void CalculateTransmissionBeamSpreader::exec() {
     throw std::invalid_argument("The input workspaces do not come from the same instrument");
   }
   // Check that the two inputs have matching binning
-  if (!WorkspaceHelpers::matchingBins(*sample_spreaderWS, *direct_spreaderWS) ||
-      !WorkspaceHelpers::matchingBins(*sample_spreaderWS, *sample_scatterWS) ||
-      !WorkspaceHelpers::matchingBins(*sample_spreaderWS, *direct_scatterWS)) {
+  if (!WorkspaceHelpers::matchingBins(sample_spreaderWS, direct_spreaderWS) ||
+      !WorkspaceHelpers::matchingBins(sample_spreaderWS, sample_scatterWS) ||
+      !WorkspaceHelpers::matchingBins(sample_spreaderWS, direct_scatterWS)) {
     g_log.error("Input workspaces do not have matching binning");
     throw std::invalid_argument("Input workspaces do not have matching binning");
   }

Framework/Algorithms/src/ConjoinWorkspaces.cpp

@@ -35,6 +35,8 @@ void ConjoinWorkspaces::init() {
                   "The label to set the Y axis to");
   declareProperty(std::make_unique<PropertyWithValue<std::string>>("YAxisUnit", "", Direction::Input),
                   "The unit to set the Y axis to");
+  declareProperty(std::make_unique<PropertyWithValue<bool>>("CheckMatchingBins", true, Direction::Input),
+                  "If true, the algorithm will check that the two input workspaces have matching bins.");
 }
 
 //----------------------------------------------------------------------------------------------
@@ -54,8 +56,18 @@ 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 whether bins match
+  bool checkBins = getProperty("CheckMatchingBins");
+  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.");
     g_log.error(message);
     throw std::invalid_argument(message);
   }
@@ -77,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

Framework/Algorithms/src/Divide.cpp

@@ -81,7 +81,7 @@ void Divide::setOutputUnits(const API::MatrixWorkspace_const_sptr lhs, const API
     // if both workspaces are ragged, output workspace `isDistribution` flag will be true
     out->setDistribution(true);
   }
-  if (rhs->YUnit().empty() || !WorkspaceHelpers::matchingBins(*lhs, *rhs, true)) {
+  if (rhs->YUnit().empty() || !WorkspaceHelpers::matchingBins(lhs, rhs, true)) {
     // Do nothing
   }
 
@@ -207,7 +207,7 @@ std::string Divide::checkSizeCompatibility(const API::MatrixWorkspace_const_sptr
   if (m_matchXSize) {
     // Past this point, for a 2D WS operation, we require the X arrays to match.
     // Note this only checks the first spectrum except for ragged workspaces
-    if (!WorkspaceHelpers::matchingBins(*lhs, *rhs, !m_lhsRagged && !m_rhsRagged)) {
+    if (!WorkspaceHelpers::matchingBins(lhs, rhs, !m_lhsRagged && !m_rhsRagged)) {
       return "X arrays must match when dividing 2D workspaces.";
     }
   }

Framework/Algorithms/src/MergeRuns.cpp

@@ -447,7 +447,7 @@ std::optional<std::vector<double>> MergeRuns::checkRebinning() {
   std::optional<std::vector<double>> rebinParams{std::nullopt};
   std::vector<double> bins{(*it)->x(0).rawData()};
   for (++it; it != inputsSortedByX.cend(); ++it) {
-    if (!WorkspaceHelpers::matchingBins(*inputsSortedByX.front(), **it, true)) {
+    if (!WorkspaceHelpers::matchingBins(inputsSortedByX.front(), *it, true)) {
       if (rebinBehaviour != REBIN_BEHAVIOUR) {
         if (sampleLogsFailBehaviour == SKIP_BEHAVIOUR) {
           g_log.error() << "Could not merge run: " << (*it)->getName()

Framework/Algorithms/src/Multiply.cpp

@@ -172,7 +172,7 @@ std::string Multiply::checkSizeCompatibility(const API::MatrixWorkspace_const_sp
     if (m_matchXSize) {
       // Past this point, for a 2D WS operation, we require the X arrays to
       // match. Note this only checks the first spectrum except for ragged workspaces
-      if (!WorkspaceHelpers::matchingBins(*lhs, *rhs, !m_lhsRagged && !m_rhsRagged)) {
+      if (!WorkspaceHelpers::matchingBins(lhs, rhs, !m_lhsRagged && !m_rhsRagged)) {
         return "X arrays must match when multiplying 2D workspaces.";
       }
     }

Framework/Algorithms/src/NormaliseToMonitor.cpp

@@ -467,7 +467,7 @@ MatrixWorkspace_sptr NormaliseToMonitor::getMonitorWorkspace(const MatrixWorkspa
   m_workspaceIndexes = std::vector<size_t>(1, wsID);
   // In this case we need to test whether the bins in the monitor workspace
   // match
-  m_commonBins = (m_commonBins && WorkspaceHelpers::matchingBins(*inputWorkspace, *monitorWS, true));
+  m_commonBins = (m_commonBins && WorkspaceHelpers::matchingBins(inputWorkspace, monitorWS, true));
   // Copy the monitor spectrum because it will get changed
   return monitorWS;
 }

Framework/Algorithms/src/PerformIndexOperations.cpp

@@ -37,6 +37,7 @@ class Command {
       conjoinWorkspaceAlg->initialize();
       conjoinWorkspaceAlg->setProperty("InputWorkspace1", toAppend);
       conjoinWorkspaceAlg->setProperty("InputWorkspace2", current);
+      conjoinWorkspaceAlg->setProperty("CheckMatchingBins", false);
       conjoinWorkspaceAlg->execute();
       MatrixWorkspace_sptr outWS = conjoinWorkspaceAlg->getProperty("InputWorkspace1");
       return outWS;
@@ -118,6 +119,7 @@ class CropCommand : public Command {
         conjoinWorkspaceAlg->initialize();
         conjoinWorkspaceAlg->setProperty("InputWorkspace1", outWS);
         conjoinWorkspaceAlg->setProperty("InputWorkspace2", subRange);
+        conjoinWorkspaceAlg->setProperty("CheckMatchingBins", false);
         conjoinWorkspaceAlg->execute();
         outWS = conjoinWorkspaceAlg->getProperty("InputWorkspace1");
       }

Framework/Algorithms/src/PointByPointVCorrection.cpp

@@ -158,7 +158,7 @@ void PointByPointVCorrection::check_validity(API::MatrixWorkspace_const_sptr &w1
     throw std::runtime_error("The input workspaces are not the same size");
   }
   // Now check that the bins match
-  if (!WorkspaceHelpers::matchingBins(*w1, *w2)) {
+  if (!WorkspaceHelpers::matchingBins(w1, w2)) {
     g_log.error("The input workspaces have different binning");
     throw std::runtime_error("The input workspaces have different binning");
   }

Framework/Algorithms/src/PolarizationCorrections/DepolarizedAnalyserTransmission.cpp

@@ -122,7 +122,7 @@ std::map<std::string, std::string> DepolarizedAnalyserTransmission::validateInpu
   }
   validateWorkspace(mtWs, PropNames::MT_WORKSPACE, result);
 
-  if (!WorkspaceHelpers::matchingBins(*depWs, *mtWs, true)) {
+  if (!WorkspaceHelpers::matchingBins(depWs, mtWs, true)) {
     result[PropNames::DEP_WORKSPACE] = "The bins in the " + std::string(PropNames::DEP_WORKSPACE) + " and " +
                                        PropNames::MT_WORKSPACE + " do not match.";
   }

Framework/Algorithms/src/PolarizationCorrections/PolarizationEfficienciesWildes.cpp

@@ -129,7 +129,7 @@ void PolarizationEfficienciesWildes::init() {
 namespace {
 bool hasMatchingBins(const Mantid::API::MatrixWorkspace_sptr &workspace, const Mantid::API::MatrixWorkspace_sptr &refWs,
                      const std::string &propertyName, std::map<std::string, std::string> &problems) {
-  if (!WorkspaceHelpers::matchingBins(*workspace, *refWs, true)) {
+  if (!WorkspaceHelpers::matchingBins(workspace, refWs, true)) {
     problems[propertyName] = "All input workspaces must have the same X values.";
     return false;
   }

Framework/Algorithms/src/PolarizationCorrections/PolarizerEfficiency.cpp

@@ -136,7 +136,7 @@ std::map<std::string, std::string> PolarizerEfficiency::validateInputs() {
   } else {
     const MatrixWorkspace_sptr t00Ws =
         std::dynamic_pointer_cast<MatrixWorkspace>(inputWorkspace->getItem(t00WsIndex.value()));
-    if (!WorkspaceHelpers::matchingBins(*t00Ws, *analyserWs, true)) {
+    if (!WorkspaceHelpers::matchingBins(t00Ws, analyserWs, true)) {
       errorList[PropertyNames::ANALYSER_EFFICIENCY] = "The bins in the " + std::string(PropertyNames::INPUT_WORKSPACE) +
                                                       " and " + PropertyNames::ANALYSER_EFFICIENCY +
                                                       "workspace do not match.";

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