From c719a42584aa4197fcecea46bcdee3cdc6bb5229 Mon Sep 17 00:00:00 2001
From: Matthew Andrew <matthew.andrew@tessella.com>
Date: Wed, 14 Nov 2018 15:38:28 +0000
Subject: [PATCH] Refactored MuonPairingAsymmetry Re #23642

Added child algorithms to seeAlso of MuonPairingAsymmetry
Corrected truth table of checkConsistentPeriods
Moved alnum check to helper function
Updated error message to be more verbose
Added multi period example to docs
---
 .../Muon/inc/MantidMuon/MuonAlgorithmHelper.h |  3 ++
 Framework/Muon/src/MuonAlgorithmHelper.cpp    |  4 ++
 Framework/Muon/src/MuonPairingAsymmetry.cpp   | 34 ++++++-------
 .../algorithms/MuonPairingAsymmetry-v1.rst    | 49 ++++++++++++++++++-
 4 files changed, 69 insertions(+), 21 deletions(-)

diff --git a/Framework/Muon/inc/MantidMuon/MuonAlgorithmHelper.h b/Framework/Muon/inc/MantidMuon/MuonAlgorithmHelper.h
index 3f8cd8b2b51..aa3dbf5f0cd 100644
--- a/Framework/Muon/inc/MantidMuon/MuonAlgorithmHelper.h
+++ b/Framework/Muon/inc/MantidMuon/MuonAlgorithmHelper.h
@@ -117,6 +117,9 @@ DLLExport bool checkValidPair(const std::string &name1,
 
 /// Check whether a group or pair name is valid
 DLLExport bool checkValidGroupPairName(const std::string &name);
+
+DLLExport bool is_alphanumerical_or_underscore(char character);
+
 //
 ///// Saves grouping to the XML file specified
 // DLLExport std::string groupingToXML(const Mantid::API::Grouping &grouping);
diff --git a/Framework/Muon/src/MuonAlgorithmHelper.cpp b/Framework/Muon/src/MuonAlgorithmHelper.cpp
index ee599acb6d3..2fa20284346 100644
--- a/Framework/Muon/src/MuonAlgorithmHelper.cpp
+++ b/Framework/Muon/src/MuonAlgorithmHelper.cpp
@@ -516,5 +516,9 @@ bool checkValidGroupPairName(const std::string &name) {
   return true;
 }
 
+bool is_alphanumerical_or_underscore(char character) {
+  return (isalpha(character) || isdigit(character) || (character == '_'));
+}
+
 } // namespace MuonAlgorithmHelper
 } // namespace Mantid
diff --git a/Framework/Muon/src/MuonPairingAsymmetry.cpp b/Framework/Muon/src/MuonPairingAsymmetry.cpp
index 192134b0506..daf6c75c0c1 100644
--- a/Framework/Muon/src/MuonPairingAsymmetry.cpp
+++ b/Framework/Muon/src/MuonPairingAsymmetry.cpp
@@ -39,7 +39,10 @@ int countPeriods(Workspace_sptr ws) {
 }
 
 bool checkConsistentPeriods(Workspace_sptr ws1, Workspace_sptr ws2) {
-  if (isMultiPeriod(ws1) && isMultiPeriod(ws2)) {
+  if (isMultiPeriod(ws1)) {
+    if (!isMultiPeriod(ws2)) {
+      return false
+    }
     if (countPeriods(ws1) != countPeriods(ws2)) {
       return false;
     }
@@ -52,10 +55,6 @@ MatrixWorkspace_sptr getWorkspace(WorkspaceGroup_sptr group, const int &index) {
   return boost::dynamic_pointer_cast<MatrixWorkspace>(ws);
 }
 
-bool is_alnum_underscore(char c) {
-  return (isalpha(c) || isdigit(c) || (c == '_'));
-}
-
 MatrixWorkspace_sptr groupDetectors(MatrixWorkspace_sptr workspace,
                                     const std::vector<int> &detectorIDs) {
 
@@ -65,7 +64,8 @@ MatrixWorkspace_sptr groupDetectors(MatrixWorkspace_sptr workspace,
       workspace->getIndicesFromDetectorIDs(detectorIDs);
 
   if (wsIndices.size() != detectorIDs.size())
-    throw std::invalid_argument("Some of the detector IDs were not found");
+    throw std::invalid_argument("The number of detectors requested does not equal
+     the number of detectors provided (wsIndices.size() != detectorIDs.size())");
 
   outputWS->getSpectrum(0).clearDetectorIDs();
   outputWS->setSharedX(0, workspace->sharedX(wsIndices.front()));
@@ -125,8 +125,7 @@ void MuonPairingAsymmetry::init() {
       "Alpha parameter used in the asymmetry calculation.", Direction::Input);
 
   declareProperty("SpecifyGroupsManually", false,
-                  "Specify the pair of groups manually using the raw data and "
-                  "various optional parameters.");
+                  "Specify the pair of groups manually");
 
   // Select groups via workspaces
 
@@ -213,7 +212,7 @@ std::map<std::string, std::string> MuonPairingAsymmetry::validateInputs() {
     errors["PairName"] = "Pair name must be specified.";
   }
   if (!std::all_of(std::begin(pairName), std::end(pairName),
-                   is_alnum_underscore)) {
+                   MuonAlgorithmHelper::is_alphanumerical_or_underscore)) {
     errors["PairName"] =
         "The pair name must contain alphnumeric characters and _ only.";
   }
@@ -310,7 +309,7 @@ MatrixWorkspace_sptr MuonPairingAsymmetry::execGroupWorkspaceInput() {
   const double alpha = static_cast<double>(getProperty("Alpha"));
   std::vector<int> summedPeriods = getProperty("SummedPeriods");
   std::vector<int> subtractedPeriods = getProperty("SubtractedPeriods");
-  return calcAsymmetryWithSummedAndSubtractedPeriods(
+  return calcPairAsymmetryWithSummedAndSubtractedPeriods(
       summedPeriods, subtractedPeriods, groupedPeriods, alpha);
 }
 
@@ -324,12 +323,12 @@ MatrixWorkspace_sptr MuonPairingAsymmetry::execSpecifyGroupsManually() {
   std::vector<int> subtractedPeriods = getProperty("SubtractedPeriods");
   const double alpha = static_cast<double>(getProperty("Alpha"));
 
-  return calcAsymmetryWithSummedAndSubtractedPeriods(
+  return calcPairAsymmetryWithSummedAndSubtractedPeriods(
       summedPeriods, subtractedPeriods, groupedPeriods, alpha);
 }
 
 MatrixWorkspace_sptr
-MuonPairingAsymmetry::calcAsymmetryWithSummedAndSubtractedPeriods(
+MuonPairingAsymmetry::calcPairAsymmetryWithSummedAndSubtractedPeriods(
     const std::vector<int> &summedPeriods,
     const std::vector<int> &subtractedPeriods,
     WorkspaceGroup_sptr groupedPeriods, const double &alpha) {
@@ -338,14 +337,14 @@ MuonPairingAsymmetry::calcAsymmetryWithSummedAndSubtractedPeriods(
   auto subtractedWS =
       MuonAlgorithmHelper::sumPeriods(groupedPeriods, subtractedPeriods);
 
-  MatrixWorkspace_sptr asymSummedPeriods = asymmetryCalc(summedWS, alpha);
+  MatrixWorkspace_sptr asymSummedPeriods = pairAsymmetryCalc(summedWS, alpha);
 
   if (subtractedPeriods.empty()) {
     return asymSummedPeriods;
   }
 
   MatrixWorkspace_sptr asymSubtractedPeriods =
-      asymmetryCalc(subtractedWS, alpha);
+      pairAsymmetryCalc(subtractedWS, alpha);
 
   return MuonAlgorithmHelper::subtractWorkspaces(asymSummedPeriods,
                                                  asymSubtractedPeriods);
@@ -380,7 +379,7 @@ MuonPairingAsymmetry::createGroupWorkspace(WorkspaceGroup_sptr inputWS) {
  * @returns MatrixWorkspace containing result of the calculation.
  */
 MatrixWorkspace_sptr
-MuonPairingAsymmetry::asymmetryCalc(MatrixWorkspace_sptr inputWS,
+MuonPairingAsymmetry::pairAsymmetryCalc(MatrixWorkspace_sptr inputWS,
                                     const double &alpha) {
   MatrixWorkspace_sptr outWS;
 
@@ -390,7 +389,7 @@ MuonPairingAsymmetry::asymmetryCalc(MatrixWorkspace_sptr inputWS,
   std::vector<int> fwdSpectra = {0};
   std::vector<int> bwdSpectra = {1};
 
-  IAlgorithm_sptr alg = this->createChildAlgorithm("AsymmetryCalc");
+  IAlgorithm_sptr alg = this->createChildAlgorithm("pairAsymmetryCalc");
   alg->setChild(true);
   alg->setProperty("InputWorkspace", inputWS);
   alg->setProperty("ForwardSpectra", fwdSpectra);
@@ -469,8 +468,5 @@ void MuonPairingAsymmetry::validatePeriods(
   }
 }
 
-// Allow WorkspaceGroup property to function correctly.
-bool MuonPairingAsymmetry::checkGroups() { return false; }
-
 } // namespace Muon
 } // namespace Mantid
diff --git a/docs/source/algorithms/MuonPairingAsymmetry-v1.rst b/docs/source/algorithms/MuonPairingAsymmetry-v1.rst
index eab23067320..fb55ea9fe31 100644
--- a/docs/source/algorithms/MuonPairingAsymmetry-v1.rst
+++ b/docs/source/algorithms/MuonPairingAsymmetry-v1.rst
@@ -22,7 +22,7 @@ With a pair, one may define an asymmetry operation as in :ref:`algm-AsymmetryCal
 
 .. math:: A = \frac{F-\alpha B}{F+\alpha B},
 
-where :math:`F` and :math:`B` are the two groups.
+where :math:`F` and :math:`B` are the forward and backwards groups and alpha is the balance parameter.
 
 The pair must be given a name via **PairName** which can consist of letters, numbers and underscores. 
 
@@ -31,7 +31,7 @@ The pair must be given a name via **PairName** which can consist of letters, num
 
 The pair name does not affect the data; however the name is used in the muon interface when automatically generating workspace names from group data.
 
-Additionally, a value for **Alpha** must be supplied, and which must be non-negative.
+Additionally, a value for **alpha** must be supplied, and which must be non-negative.
 
 There are two options for supplying the group data :
 
@@ -93,6 +93,51 @@ Output:
 	X values are : [0.0, 1.0, 2.0, 3.0, 4.0, 5.0]
     Y values are : [-0.4, -0.286, -0.222, -0.286, -0.4]
 
+**Example - Using MuonPreProcess and Specifying Groups Manually for Multi Period Data**
+
+.. testcode:: SpecifyGroupsManuallyMultiPeriod
+
+    # Create a workspaces with four spectra
+    dataX = [0, 1, 2, 3, 4, 5] * 4
+    dataY = [10, 20, 30, 20, 10] + \
+            [20, 30, 40, 30, 20] + \
+            [30, 40, 50, 40, 30] + \
+            [40, 50, 60, 50, 40]
+    print(dataY)
+    input_workspace = CreateWorkspace(dataX, dataY, NSpec=4)
+    input_workspace_1 = CreateWorkspace(dataX, dataY, NSpec=4)
+    for i in range(4):
+        # set detector IDs to be 1,2,3,4
+        # these do not have to be the same as the spectrum numbers
+        # (the spectrum number are 0,1,2,3 in this case)
+        input_workspace.getSpectrum(i).setDetectorID(i + 1)
+        input_workspace_1.getSpectrum(i).setDetectorID(i + 1)
+
+    # Create multi period data
+    multi_period_data = GroupWorkspaces(input_workspace)
+    multi_period_data.addWorkspace(input_workspace_1)
+
+    pre_processed_workspace = MuonPreProcess(InputWorkspace=input_workspace)
+
+    output_workspace = MuonPairingAsymmetry(InputWorkspace=pre_processed_workspace,
+                                                      PairName="myPair",
+                                                      Alpha=1.0,
+                                                      SpecifyGroupsManually=True,
+                                                      Group1=[1, 2],
+                                                      Group2=[3, 4],
+                                                      SummedPeriods=[1, 2])
+
+    print("X values are : {}".format([round(float(i), 3) for i in output_workspace.readX(0)]))
+    print("Y values are : {}".format([round(float(i), 3) for i in output_workspace.readY(0)]))
+
+
+Output:
+
+.. testoutput:: SpecifyGroupsManuallyMultiPeriod
+
+	X values are : [0.0, 1.0, 2.0, 3.0, 4.0, 5.0]
+    Y values are : [-0.4, -0.286, -0.222, -0.286, -0.4]
+
 **Example - Using MuonPreProcess, MuonGroupingCounts for Single Period Data**
 
 .. testcode:: SinglePeriod
-- 
GitLab