From 6d83943be88947c5698b6edfec0467e540aeb9da Mon Sep 17 00:00:00 2001
From: Simon Heybrock <simon.heybrock@esss.se>
Date: Fri, 17 Nov 2017 10:33:02 +0100
Subject: [PATCH] Re #21181. MPI support for BinaryOperation and subclasses.

---
 .../inc/MantidAlgorithms/BinaryOperation.h    |   8 +-
 Framework/Algorithms/src/BinaryOperation.cpp  |  19 ++
 .../Algorithms/test/BinaryOperationTest.h     | 178 +++++++++++++++++-
 .../development/AlgorithmMPISupport.rst       |  13 +-
 4 files changed, 207 insertions(+), 11 deletions(-)

diff --git a/Framework/Algorithms/inc/MantidAlgorithms/BinaryOperation.h b/Framework/Algorithms/inc/MantidAlgorithms/BinaryOperation.h
index d0acce92ddb..6dfa0e3120b 100644
--- a/Framework/Algorithms/inc/MantidAlgorithms/BinaryOperation.h
+++ b/Framework/Algorithms/inc/MantidAlgorithms/BinaryOperation.h
@@ -1,7 +1,7 @@
 #ifndef MANTID_ALGORITHMS_BINARYOPERATION_H_
 #define MANTID_ALGORITHMS_BINARYOPERATION_H_
 
-#include "MantidAPI/ParallelAlgorithm.h"
+#include "MantidAPI/Algorithm.h"
 #include "MantidAPI/Run.h"
 #include "MantidAPI/SpectrumInfo.h"
 #include "MantidAPI/WorkspaceGroup_fwd.h"
@@ -53,7 +53,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 File change history is stored at: <https://github.com/mantidproject/mantid>
 */
-class DLLExport BinaryOperation : public API::ParallelAlgorithm {
+class DLLExport BinaryOperation : public API::Algorithm {
 public:
   /// Algorithm's category for identification overriding a virtual method
   const std::string category() const override { return "Arithmetic"; }
@@ -71,6 +71,10 @@ public:
                             const API::MatrixWorkspace_const_sptr &rhs);
 
 protected:
+  Parallel::ExecutionMode getParallelExecutionMode(
+      const std::map<std::string, Parallel::StorageMode> &storageModes)
+      const override;
+
   // Overridden Algorithm methods
   void exec() override;
   void init() override;
diff --git a/Framework/Algorithms/src/BinaryOperation.cpp b/Framework/Algorithms/src/BinaryOperation.cpp
index cd08daf1ffe..39c1a39ffd8 100644
--- a/Framework/Algorithms/src/BinaryOperation.cpp
+++ b/Framework/Algorithms/src/BinaryOperation.cpp
@@ -1034,5 +1034,24 @@ BinaryOperation::buildBinaryOperationTable(
   return table;
 }
 
+Parallel::ExecutionMode BinaryOperation::getParallelExecutionMode(
+    const std::map<std::string, Parallel::StorageMode> &storageModes) const {
+  if (static_cast<bool>(getProperty("AllowDifferentNumberSpectra")))
+    return Parallel::ExecutionMode::Invalid;
+  auto lhs = storageModes.find("LHSWorkspace")->second;
+  auto rhs = storageModes.find("RHSWorkspace")->second;
+  // Two identical modes is ok
+  if (lhs == rhs)
+    return getCorrespondingExecutionMode(storageModes.begin()->second);
+  // Mode <X> times Cloned is ok if the cloned workspace is WorkspaceSingleValue
+  if (rhs == Parallel::StorageMode::Cloned) {
+    API::MatrixWorkspace_const_sptr ws = getProperty("RHSWorkspace");
+    if (boost::dynamic_pointer_cast<const WorkspaceSingleValue>(ws))
+      return getCorrespondingExecutionMode(lhs);
+  }
+  // Other options are not ok (e.g., MasterOnly times Distributed)
+  return Parallel::ExecutionMode::Invalid;
+}
+
 } // namespace Algorithms
 } // namespace Mantid
diff --git a/Framework/Algorithms/test/BinaryOperationTest.h b/Framework/Algorithms/test/BinaryOperationTest.h
index 10f767a5d1e..d7933a7d311 100644
--- a/Framework/Algorithms/test/BinaryOperationTest.h
+++ b/Framework/Algorithms/test/BinaryOperationTest.h
@@ -4,6 +4,8 @@
 #include <cxxtest/TestSuite.h>
 #include <cmath>
 
+#include "MantidTestHelpers/ParallelAlgorithmCreation.h"
+#include "MantidTestHelpers/ParallelRunner.h"
 #include "MantidTestHelpers/WorkspaceCreationHelper.h"
 #include "MantidAlgorithms/BinaryOperation.h"
 #include "MantidAPI/AnalysisDataService.h"
@@ -11,21 +13,18 @@
 #include "MantidAPI/WorkspaceFactory.h"
 #include "MantidDataObjects/EventWorkspace.h"
 #include "MantidDataObjects/Workspace2D.h"
+#include "MantidDataObjects/WorkspaceCreation.h"
 #include "MantidKernel/Timer.h"
+#include "MantidIndexing/IndexInfo.h"
 
+using namespace Mantid;
 using namespace Mantid::API;
 using namespace Mantid::Algorithms;
 using namespace Mantid::DataObjects;
-using Mantid::DataObjects::Workspace2D_sptr;
 using Mantid::Geometry::IDetector_const_sptr;
 
 class BinaryOpHelper : public Mantid::Algorithms::BinaryOperation {
 public:
-  /// Default constructor
-  BinaryOpHelper() : BinaryOperation(){};
-  /// Destructor
-  ~BinaryOpHelper() override{};
-
   /// function to return a name of the algorithm, must be overridden in all
   /// algorithms
   const std::string name() const override { return "BinaryOpHelper"; }
@@ -64,6 +63,117 @@ private:
                               Mantid::MantidVec &) override {}
 };
 
+namespace {
+void run_parallel(const Parallel::Communicator &comm,
+                  const Parallel::StorageMode storageMode) {
+  using namespace Parallel;
+  auto alg = ParallelTestHelpers::create<BinaryOpHelper>(comm);
+  if (comm.rank() == 0 || storageMode != StorageMode::MasterOnly) {
+    Indexing::IndexInfo indexInfo(100, storageMode, comm);
+    alg->setProperty("LHSWorkspace",
+                     create<Workspace2D>(indexInfo, HistogramData::Points(1)));
+    alg->setProperty("RHSWorkspace",
+                     create<Workspace2D>(indexInfo, HistogramData::Points(1)));
+  } else {
+    alg->setProperty("LHSWorkspace",
+                     Kernel::make_unique<Workspace2D>(StorageMode::MasterOnly));
+    alg->setProperty("RHSWorkspace",
+                     Kernel::make_unique<Workspace2D>(StorageMode::MasterOnly));
+  }
+  TS_ASSERT_THROWS_NOTHING(alg->execute());
+  MatrixWorkspace_const_sptr out = alg->getProperty("OutputWorkspace");
+  TS_ASSERT_EQUALS(out->storageMode(), storageMode);
+}
+
+void run_parallel_mismatch_fail(const Parallel::Communicator &comm,
+                                const Parallel::StorageMode storageModeA,
+                                const Parallel::StorageMode storageModeB) {
+  using namespace Parallel;
+  auto alg = ParallelTestHelpers::create<BinaryOpHelper>(comm);
+  if (comm.rank() == 0 || storageModeA != StorageMode::MasterOnly) {
+    Indexing::IndexInfo indexInfo(100, storageModeA, comm);
+    alg->setProperty("LHSWorkspace",
+                     create<Workspace2D>(indexInfo, HistogramData::Points(1)));
+  } else {
+    alg->setProperty("LHSWorkspace",
+                     Kernel::make_unique<Workspace2D>(StorageMode::MasterOnly));
+  }
+  if (comm.rank() == 0 || storageModeB != StorageMode::MasterOnly) {
+    Indexing::IndexInfo indexInfo(100, storageModeB, comm);
+    alg->setProperty("RHSWorkspace",
+                     create<Workspace2D>(indexInfo, HistogramData::Points(1)));
+  } else {
+    alg->setProperty("RHSWorkspace",
+                     Kernel::make_unique<Workspace2D>(StorageMode::MasterOnly));
+  }
+  if (comm.size() > 1) {
+    TS_ASSERT_THROWS_EQUALS(
+        alg->execute(), const std::runtime_error &e, std::string(e.what()),
+        "Algorithm does not support execution with input workspaces of the "
+        "following storage types: \nLHSWorkspace " +
+            toString(storageModeA) + "\nRHSWorkspace " +
+            toString(storageModeB) + "\n.");
+  } else {
+    TS_ASSERT_THROWS_NOTHING(alg->execute());
+    MatrixWorkspace_const_sptr out = alg->getProperty("OutputWorkspace");
+    TS_ASSERT_EQUALS(out->storageMode(), storageModeA);
+  }
+}
+
+void run_parallel_single_value(const Parallel::Communicator &comm,
+                               const Parallel::StorageMode storageModeA,
+                               const Parallel::StorageMode storageModeB) {
+  using namespace Parallel;
+  auto alg = ParallelTestHelpers::create<BinaryOpHelper>(comm);
+  if (comm.rank() == 0 || storageModeA != StorageMode::MasterOnly) {
+    Indexing::IndexInfo indexInfo(100, storageModeA, comm);
+    alg->setProperty("LHSWorkspace",
+                     create<Workspace2D>(indexInfo, HistogramData::Points(1)));
+  } else {
+    alg->setProperty("LHSWorkspace",
+                     Kernel::make_unique<Workspace2D>(StorageMode::MasterOnly));
+  }
+  if (comm.rank() == 0 || storageModeB != StorageMode::MasterOnly) {
+    Indexing::IndexInfo indexInfo(1, storageModeB, comm);
+    alg->setProperty("RHSWorkspace", create<WorkspaceSingleValue>(
+                                         indexInfo, HistogramData::Points(1)));
+  } else {
+    alg->setProperty("RHSWorkspace", Kernel::make_unique<WorkspaceSingleValue>(
+                                         0.0, 0.0, StorageMode::MasterOnly));
+  }
+  TS_ASSERT_THROWS_NOTHING(alg->execute());
+  MatrixWorkspace_const_sptr out = alg->getProperty("OutputWorkspace");
+  TS_ASSERT_EQUALS(out->storageMode(), storageModeA);
+}
+
+void run_parallel_AllowDifferentNumberSpectra_fail(
+    const Parallel::Communicator &comm,
+    const Parallel::StorageMode storageMode) {
+  using namespace Parallel;
+  auto alg = ParallelTestHelpers::create<BinaryOpHelper>(comm);
+  if (comm.rank() == 0 || storageMode != StorageMode::MasterOnly) {
+    Indexing::IndexInfo indexInfo(100, storageMode, comm);
+    alg->setProperty("LHSWorkspace",
+                     create<Workspace2D>(indexInfo, HistogramData::Points(1)));
+    alg->setProperty("RHSWorkspace",
+                     create<Workspace2D>(indexInfo, HistogramData::Points(1)));
+  } else {
+    alg->setProperty("LHSWorkspace",
+                     Kernel::make_unique<Workspace2D>(StorageMode::MasterOnly));
+    alg->setProperty("RHSWorkspace",
+                     Kernel::make_unique<Workspace2D>(StorageMode::MasterOnly));
+  }
+  alg->setProperty("AllowDifferentNumberSpectra", true);
+  if (comm.size() > 1) {
+    TS_ASSERT_THROWS(alg->execute(), const std::runtime_error &);
+  } else {
+    TS_ASSERT_THROWS_NOTHING(alg->execute());
+    MatrixWorkspace_const_sptr out = alg->getProperty("OutputWorkspace");
+    TS_ASSERT_EQUALS(out->storageMode(), storageMode);
+  }
+}
+}
+
 class BinaryOperationTest : public CxxTest::TestSuite {
 public:
   void testcheckSizeCompatibility1D1D() {
@@ -291,6 +401,62 @@ public:
       TS_ASSERT_EQUALS((*table)[i], i / 100);
     }
   }
+
+  void test_parallel_Distributed() {
+    ParallelTestHelpers::runParallel(run_parallel,
+                                     Parallel::StorageMode::Distributed);
+  }
+
+  void test_parallel_Cloned() {
+    ParallelTestHelpers::runParallel(run_parallel,
+                                     Parallel::StorageMode::Cloned);
+  }
+
+  void test_parallel_MasterOnly() {
+    ParallelTestHelpers::runParallel(run_parallel,
+                                     Parallel::StorageMode::MasterOnly);
+  }
+
+  void test_parallel_mistmatch_fail() {
+    using ParallelTestHelpers::runParallel;
+    auto cloned = Parallel::StorageMode::Cloned;
+    auto distri = Parallel::StorageMode::Distributed;
+    auto master = Parallel::StorageMode::MasterOnly;
+    runParallel(run_parallel_mismatch_fail, cloned, distri);
+    runParallel(run_parallel_mismatch_fail, cloned, master);
+    runParallel(run_parallel_mismatch_fail, distri, cloned);
+    runParallel(run_parallel_mismatch_fail, distri, master);
+    runParallel(run_parallel_mismatch_fail, master, cloned);
+    runParallel(run_parallel_mismatch_fail, master, distri);
+  }
+
+  void test_parallel_Cloned_ClonedSingle() {
+    ParallelTestHelpers::runParallel(run_parallel_single_value,
+                                     Parallel::StorageMode::Cloned,
+                                     Parallel::StorageMode::Cloned);
+  }
+
+  void test_parallel_Distributed_ClonedSingle() {
+    ParallelTestHelpers::runParallel(run_parallel_single_value,
+                                     Parallel::StorageMode::Distributed,
+                                     Parallel::StorageMode::Cloned);
+  }
+
+  void test_parallel_MasterOnly_ClonedSingle() {
+    ParallelTestHelpers::runParallel(run_parallel_single_value,
+                                     Parallel::StorageMode::MasterOnly,
+                                     Parallel::StorageMode::Cloned);
+  }
+
+  void test_parallel_AllowDifferentNumberSpectra_fail() {
+    using ParallelTestHelpers::runParallel;
+    runParallel(run_parallel_AllowDifferentNumberSpectra_fail,
+                Parallel::StorageMode::Cloned);
+    runParallel(run_parallel_AllowDifferentNumberSpectra_fail,
+                Parallel::StorageMode::Distributed);
+    runParallel(run_parallel_AllowDifferentNumberSpectra_fail,
+                Parallel::StorageMode::MasterOnly);
+  }
 };
 
 #endif /*BINARYOPERATIONTEST_H_*/
diff --git a/docs/source/development/AlgorithmMPISupport.rst b/docs/source/development/AlgorithmMPISupport.rst
index 4c14e9404ab..8858b257d5e 100644
--- a/docs/source/development/AlgorithmMPISupport.rst
+++ b/docs/source/development/AlgorithmMPISupport.rst
@@ -454,15 +454,17 @@ Supported Algorithms
 ================================= =============== ========
 Algorithm                         Supported modes Comments
 ================================= =============== ========
+BinaryOperation                   all             not supported if ``AllowDifferentNumberSpectra`` is enabled
 CloneWorkspace                    all
 CompressEvents                    all
-CreateWorkspace                   all
 CreateSingleValuedWorkspace       Identical       ``OutputWorkspace`` has ``StorageMode::Cloned``, support of ``MasterOnly`` would require adding property for selecting the mode
+CreateWorkspace                   all
 CropToComponent                   all
-CropWorkspace                     all             see ExtractSpectra regarding X cropping
+CropWorkspace                     all             see ``ExtractSpectra`` regarding X cropping
+Divide                            all             see ``BinaryOperation``
+ExtractSingleSpectrum             all             in practice ``ExecutionMode::Distributed`` not supported due to current nonzero-spectrum-count limitation
 ExtractSpectra2                   all             currently not available via algorithm factory or Python
 ExtractSpectra                    all             not supported with ``DetectorList``, cropping in X may exhibit inconsistent behavior in case spectra have common boundaries within some ranks but not within all ranks or across ranks
-ExtractSingleSpectrum             all             in practice ``ExecutionMode::Distributed`` not supported due to current nonzero-spectrum-count limitation
 FilterBadPulses                   all
 FilterByLogValue                  all
 FilterByTime                      all
@@ -471,12 +473,17 @@ LoadInstrument                    all
 LoadNexusLogs                     all
 LoadParameterFile                 all             segfaults when used in unit tests with MPI threading backend due to `#9365 <https://github.com/mantidproject/mantid/issues/9365>`_, normal use should be ok
 MaskBins                          all
+Minus                             all             see ``BinaryOperation``
 MoveInstrumentComponent           all
+Multiply                          all             see ``BinaryOperation``
+Plus                              all             see ``BinaryOperation``
+PoissonErrors                     all             see ``BinaryOperation``
 Rebin                             all             min and max bin boundaries must be given explicitly
 RebinToWorkspace                  all             ``WorkspaceToMatch`` must have ``StorageMode::Cloned``
 RemovePromptPulse                 all
 RotateInstrumentComponent         all
 SortEvents                        all
+WeightedMean                      all             see ``BinaryOperation``
 ================================= =============== ========
 
 Currently none of the above algorithms works with ``StorageMode::Distributed`` in case there are zero spectra on any rank.
-- 
GitLab