From 98d4b45b5d21fa43d8255f131358e3d74872301e Mon Sep 17 00:00:00 2001
From: Verena Reimund <reimund@ill.eu>
Date: Tue, 17 Apr 2018 11:23:08 +0200
Subject: [PATCH] Add input validation & clean doxygen commands
Refs #22197
---
.../inc/MantidAlgorithms/Stitch1D.h | 2 +
Framework/Algorithms/src/Stitch1D.cpp | 83 ++++++++++---------
Framework/Algorithms/test/Stitch1DTest.h | 65 ++++++++++-----
3 files changed, 89 insertions(+), 61 deletions(-)
diff --git a/Framework/Algorithms/inc/MantidAlgorithms/Stitch1D.h b/Framework/Algorithms/inc/MantidAlgorithms/Stitch1D.h
index beaba343583..30b3c669624 100644
--- a/Framework/Algorithms/inc/MantidAlgorithms/Stitch1D.h
+++ b/Framework/Algorithms/inc/MantidAlgorithms/Stitch1D.h
@@ -48,6 +48,8 @@ public:
}
/// Does the x-axis have non-zero errors
bool hasNonzeroErrors(Mantid::API::MatrixWorkspace_sptr ws);
+ /// Cross-check properties with each other @see IAlgorithm::validateInputs
+ std::map<std::string, std::string> validateInputs() override;
private:
/// Helper typedef. For storing indexes of special values per spectra per
diff --git a/Framework/Algorithms/src/Stitch1D.cpp b/Framework/Algorithms/src/Stitch1D.cpp
index 44df37e45a2..45a42a7540f 100644
--- a/Framework/Algorithms/src/Stitch1D.cpp
+++ b/Framework/Algorithms/src/Stitch1D.cpp
@@ -3,25 +3,24 @@
#include "MantidAPI/WorkspaceProperty.h"
#include "MantidAPI/MatrixWorkspace.h"
#include "MantidAPI/WorkspaceFactory.h"
+#include "MantidHistogramData/HistogramE.h"
#include "MantidHistogramData/HistogramX.h"
#include "MantidHistogramData/HistogramY.h"
-#include "MantidHistogramData/HistogramE.h"
#include "MantidKernel/ArrayProperty.h"
#include "MantidKernel/BoundedValidator.h"
#include "MantidKernel/MultiThreaded.h"
#include "MantidKernel/PropertyWithValue.h"
#include "MantidKernel/RebinParamsValidator.h"
+#include <algorithm>
#include <boost/tuple/tuple.hpp>
#include <boost/format.hpp>
-#include <boost/algorithm/string.hpp>
#include <boost/math/special_functions.hpp>
-#include <algorithm>
-using namespace Mantid::Kernel;
using namespace Mantid::API;
-using Mantid::HistogramData::HistogramY;
+using namespace Mantid::Kernel;
using Mantid::HistogramData::HistogramE;
+using Mantid::HistogramData::HistogramY;
namespace {
@@ -37,9 +36,7 @@ bool isNonzero(double i) { return (0 != i); }
namespace Mantid {
namespace Algorithms {
-/**
- * Range tolerance
- *
+/** Range tolerance
* This is required for machine precision reasons. Used to adjust StartOverlap
*and EndOverlap so that they are
* inclusive of bin boundaries if they are sitting ontop of the bin boundaries.
@@ -48,8 +45,7 @@ const double Stitch1D::range_tolerance = 1e-9;
// Register the algorithm into the AlgorithmFactory
DECLARE_ALGORITHM(Stitch1D)
-/**
- * Zero out all y and e data that is not in the region a1 to a2.
+/** Zero out all y and e data that is not in the region a1 to a2.
* @param a1 : Zero based bin index (first one)
* @param a2 : Zero based bin index (last one inclusive)
* @param source : Workspace providing the source data.
@@ -87,8 +83,8 @@ MatrixWorkspace_sptr Stitch1D::maskAllBut(int a1, int a2,
return product;
}
-/**
- * Mask out data in the region between a1 and a2 with zeros. Operation performed
+/** Mask out data in the region between a1 and a2 with zeros. Operation
+ * performed
* on the original workspace
* @param a1 : start position in X
* @param a2 : end position in X
@@ -123,7 +119,8 @@ void Stitch1D::init() {
"LHS input workspace.");
declareProperty(make_unique<WorkspaceProperty<MatrixWorkspace>>(
"RHSWorkspace", "", Direction::Input),
- "RHS input workspace.");
+ "RHS input workspace, must be same type as LHSWorkspace "
+ "(histogram or point data).");
declareProperty(make_unique<WorkspaceProperty<MatrixWorkspace>>(
"OutputWorkspace", "", Direction::Output),
"Output stitched workspace.");
@@ -158,7 +155,21 @@ void Stitch1D::init() {
"The actual used value for the scaling factor.");
}
-/**Gets the start of the overlapping region
+/** Validate the algorithm's properties.
+ * @return A map of porperty names and their issues.
+ */
+std::map<std::string, std::string> Stitch1D::validateInputs(void) {
+ std::map<std::string, std::string> issues;
+ MatrixWorkspace_sptr lhs = getProperty("LHSWorkspace");
+ MatrixWorkspace_sptr rhs = getProperty("RHSWorkspace");
+ if (lhs->isHistogramData() && !rhs->isHistogramData())
+ issues["RHSWorkspace"] = "Must be a histogram like LHSWorkspace.";
+ if (!lhs->isHistogramData() && rhs->isHistogramData())
+ issues["RHSWorkspace"] = "Must be point data like LHSWorkspace.";
+ return issues;
+}
+
+/** Gets the start of the overlapping region
@param intesectionMin :: The minimum possible value for the overlapping region
to inhabit
@param intesectionMax :: The maximum possible value for the overlapping region
@@ -189,7 +200,7 @@ double Stitch1D::getStartOverlap(const double &intesectionMin,
return startOverlapVal;
}
-/**Gets the end of the overlapping region
+/** Gets the end of the overlapping region
@param intesectionMin :: The minimum possible value for the overlapping region
to inhabit
@param intesectionMax :: The maximum possible value for the overlapping region
@@ -220,7 +231,7 @@ double Stitch1D::getEndOverlap(const double &intesectionMin,
return endOverlapVal;
}
-/**Gets the rebinning parameters and calculates any missing values
+/** Gets the rebinning parameters and calculates any missing values
@param lhsWS :: The left hand side input workspace
@param rhsWS :: The right hand side input workspace
@param scaleRHS :: Scale the right hand side workspace
@@ -276,7 +287,7 @@ std::vector<double> Stitch1D::getRebinParams(MatrixWorkspace_sptr &lhsWS,
return result;
}
-/**Runs the Rebin Algorithm as a child and replaces special values
+/** Runs the Rebin Algorithm as a child and replaces special values
@param input :: The input workspace
@param params :: a vector<double> containing rebinning parameters
@return A shared pointer to the resulting MatrixWorkspace
@@ -334,7 +345,7 @@ MatrixWorkspace_sptr Stitch1D::rebin(MatrixWorkspace_sptr &input,
return outWS;
}
-/**Runs the Integration Algorithm as a child.
+/** Runs the Integration Algorithm as a child.
@param input :: The input workspace
@param start :: a double defining the start of the region to integrate
@param stop :: a double defining the end of the region to integrate
@@ -356,7 +367,7 @@ MatrixWorkspace_sptr Stitch1D::integration(MatrixWorkspace_sptr &input,
return outWS;
}
-/**Runs the WeightedMean Algorithm as a child
+/** Runs the WeightedMean Algorithm as a child
@param inOne :: The first input workspace
@param inTwo :: The second input workspace
@return A shared pointer to the resulting MatrixWorkspace
@@ -371,7 +382,7 @@ MatrixWorkspace_sptr Stitch1D::weightedMean(MatrixWorkspace_sptr &inOne,
return outWS;
}
-/**Runs the ConjoinXRuns Algorithm as a child
+/** Runs the ConjoinXRuns Algorithm as a child
@param inWS :: Input workspace
@return A shared pointer to the resulting MatrixWorkspace
*/
@@ -390,7 +401,7 @@ MatrixWorkspace_sptr Stitch1D::conjoinXAxis(MatrixWorkspace_sptr &inOne,
return outWS;
}
-/**Runs the SortXAxis Algorithm as a child
+/** Runs the SortXAxis Algorithm as a child
@param inWS :: Input workspace
@return A shared pointer to the resulting MatrixWorkspace
*/
@@ -402,7 +413,7 @@ MatrixWorkspace_sptr Stitch1D::sortXAxis(MatrixWorkspace_sptr &inWS) {
return outWS;
}
-/**Runs the CreateSingleValuedWorkspace Algorithm as a child
+/** Runs the CreateSingleValuedWorkspace Algorithm as a child
@param val :: The double to convert to a single value workspace
@return A shared pointer to the resulting MatrixWorkspace
*/
@@ -415,7 +426,7 @@ MatrixWorkspace_sptr Stitch1D::singleValueWS(double val) {
return outWS;
}
-/**finds the bins containing the ends of the overlapping region
+/** Finds the bins containing the ends of the overlapping region
@param startOverlap :: The start of the overlapping region
@param endOverlap :: The end of the overlapping region
@param workspace :: The workspace to determine the overlaps inside
@@ -435,7 +446,7 @@ Stitch1D::findStartEndIndexes(double startOverlap, double endOverlap,
return boost::tuple<int, int>(a1, a2);
}
-/**Determines if a workspace has non zero errors
+/** Determines if a workspace has non zero errors
@param ws :: The input workspace
@return True if there are any non-zero errors in the workspace
*/
@@ -499,7 +510,7 @@ void Stitch1D::exec() {
if (startOverlap < xMin) {
std::string message = boost::str(
- boost::format("Stitch1D StartOverlap is outside the available X range "
+ boost::format("Stitch1D StartOverlap is outside the available X range. "
"after rebinning. StartOverlap: %10.9f, X min: %10.9f") %
startOverlap % xMin);
throw std::runtime_error(message);
@@ -509,7 +520,6 @@ void Stitch1D::exec() {
boost::format("Stitch1D EndOverlap is outside the available X range "
"after rebinning. EndOverlap: %10.9f, X max: %10.9f") %
endOverlap % xMax);
-
throw std::runtime_error(message);
}
@@ -526,7 +536,7 @@ void Stitch1D::exec() {
MatrixWorkspace_sptr lhs, rhs;
// If the input workspaces are histograms ...
- if (rhsWS->isHistogramData() && lhsWS->isHistogramData()) {
+ if (lhsWS->isHistogramData()) {
lhs = rebin(lhsWS, params);
rhs = rebin(rhsWS, params);
}
@@ -580,7 +590,7 @@ void Stitch1D::exec() {
MatrixWorkspace_sptr overlapave;
// If the input workspaces are histograms ...
- if (rhsWS->isHistogramData() && lhsWS->isHistogramData()) {
+ if (lhsWS->isHistogramData()) {
if (hasNonzeroErrors(overlap1) && hasNonzeroErrors(overlap2)) {
overlapave = weightedMean(overlap1, overlap2);
} else {
@@ -589,17 +599,9 @@ void Stitch1D::exec() {
MatrixWorkspace_sptr denominator = singleValueWS(2.0);
overlapave = sum / denominator;
}
- } else {
- // If the input workspaces are point data ...
- if (!rhsWS->isHistogramData() && !lhsWS->isHistogramData()) {
- // Sort according to x values
- auto ws = conjoinXAxis(overlap1, overlap2);
- overlapave = sortXAxis(ws);
- } else {
- g_log.error("Input workspaces must be both histograms or point data");
- throw std::invalid_argument(
- "Input workspaces must be both histograms or point data");
- }
+ } else { // The input workspaces are point data ... join & sort
+ auto ws = conjoinXAxis(overlap1, overlap2);
+ overlapave = sortXAxis(ws);
}
MatrixWorkspace_sptr result = lhs + overlapave + rhs;
@@ -615,8 +617,7 @@ void Stitch1D::exec() {
setProperty("OutScaleFactor", scaleFactor);
}
-/**
- * Put special values back.
+/** Put special values back.
* @param ws : MatrixWorkspace to resinsert special values into.
*/
void Stitch1D::reinsertSpecialValues(MatrixWorkspace_sptr ws) {
diff --git a/Framework/Algorithms/test/Stitch1DTest.h b/Framework/Algorithms/test/Stitch1DTest.h
index e7c78f32086..560b20d52c2 100644
--- a/Framework/Algorithms/test/Stitch1DTest.h
+++ b/Framework/Algorithms/test/Stitch1DTest.h
@@ -20,13 +20,13 @@
#include <boost/make_shared.hpp>
using namespace Mantid::API;
+using namespace Mantid::DataObjects;
using namespace Mantid::Kernel;
using Mantid::Algorithms::Stitch1D;
-using namespace Mantid::DataObjects;
-using Mantid::HistogramData::HistogramX;
-using Mantid::HistogramData::HistogramY;
using Mantid::HistogramData::HistogramDx;
using Mantid::HistogramData::HistogramE;
+using Mantid::HistogramData::HistogramX;
+using Mantid::HistogramData::HistogramY;
using Mantid::HistogramData::LinearGenerator;
double roundSix(double i) { return floor(i * 1000000. + 0.5) / 1000000.; }
@@ -44,8 +44,7 @@ MatrixWorkspace_sptr createWorkspace(const HistogramX &xData,
outWS->mutableY(i) = yData;
outWS->mutableE(i) = eData;
outWS->mutableX(i) = xData;
- outWS->setPointStandardDeviations(i, yData.size());
- outWS->mutableDx(i) = Dx;
+ outWS->setPointStandardDeviations(i, Dx);
}
outWS->getAxis(0)->unit() = UnitFactory::Instance().create("Wavelength");
@@ -240,22 +239,49 @@ public:
std::runtime_error &);
}
- void test_lhsworkspace_must_be_histogram() {
- auto lhs_ws = make_arbitrary_point_ws();
- auto rhs_ws = make_arbitrary_histogram_ws();
- TSM_ASSERT_THROWS(
- "Both ws must be either histogram or point data",
- do_stitch1D(lhs_ws, rhs_ws, -1., 1., std::vector<double>(1., 0.2)),
- std::invalid_argument &);
+ void test_point_workspaces_pass() {
+ auto point_ws = make_arbitrary_point_ws();
+ const auto &x = HistogramX(3, LinearGenerator(-.5, 0.2));
+ const auto &y = HistogramY(3, LinearGenerator(1., 1.0));
+ const auto &e = HistogramE(3, 1.);
+ const auto &dx = HistogramDx(3, LinearGenerator(-3., 0.1));
+ auto point_ws_2 = createWorkspace(x, y, e, dx);
+ TSM_ASSERT_THROWS_NOTHING("Point workspaces should pass",
+ do_stitch1D(point_ws, point_ws_2));
}
- void test_rhsworkspace_must_be_histogram() {
- auto lhs_ws = make_arbitrary_histogram_ws();
- auto rhs_ws = make_arbitrary_point_ws();
- TSM_ASSERT_THROWS(
- "Both ws must be either histogram or point data",
- do_stitch1D(lhs_ws, rhs_ws, -1., 1., std::vector<double>(1., 0.2)),
- std::invalid_argument &);
+ void test_histogram_workspaces_pass() {
+ auto histo_ws = make_arbitrary_histogram_ws();
+ const auto &x = HistogramX(3, LinearGenerator(-1.2, 0.2));
+ const auto &y = HistogramY(2, LinearGenerator(1., 1.0));
+ const auto &e = HistogramE(2, 1.);
+ const auto &dx = HistogramDx(2, LinearGenerator(-3., 0.1));
+ auto histo_ws_2 = createWorkspace(x, y, e, dx);
+ TSM_ASSERT_THROWS_NOTHING("Histogram workspaces should pass",
+ do_stitch1D(histo_ws_2, histo_ws));
+ }
+
+ void test_input_validation() {
+ Stitch1D alg;
+ alg.setChild(true);
+ alg.setRethrows(true);
+ alg.initialize();
+ alg.setProperty("LHSWorkspace", make_arbitrary_point_ws());
+ alg.setProperty("RHSWorkspace", make_arbitrary_histogram_ws());
+ alg.setProperty("StartOverlap", -1);
+ alg.setProperty("EndOverlap", 1);
+ alg.setProperty("Params", std::vector<double>(1., 0.2));
+ alg.setProperty("ScaleRHSWorkspace", true);
+ alg.setPropertyValue("OutputWorkspace", "dummy_value");
+ alg.execute();
+ TSM_ASSERT("RHSWorkspace must be point data.",
+ !alg.isExecuted());
+
+ alg.setProperty("LHSWorkspace", make_arbitrary_histogram_ws());
+ alg.setProperty("RHSWorkspace", make_arbitrary_point_ws());
+ alg.execute();
+ TSM_ASSERT("RHSWorkspace must be a histogram.",
+ !alg.isExecuted());
}
void test_stitching_uses_supplied_params() {
@@ -549,7 +575,6 @@ public:
TSM_ASSERT("All error values are non-zero", alg.hasNonzeroErrors(ws));
// Run it again with all zeros
-
e = HistogramE(9, 0.);
ws = createWorkspace(x, y, e, dx, nspectrum);
TSM_ASSERT("All error values are non-zero", !alg.hasNonzeroErrors(ws));
--
GitLab