From beb87a2c2611c47b1972a4791df57e3cf84a65d9 Mon Sep 17 00:00:00 2001
From: Danny Hindson <danny.hindson@stfc.ac.uk>
Date: Thu, 23 Jan 2020 10:30:43 +0000
Subject: [PATCH] Incorporate review comment relating to debug logging
Logging stats are only generated now if the logging level is debug
---
.../SampleCorrections/MCAbsorptionStrategy.h | 6 +-
.../SampleCorrections/MCInteractionVolume.h | 8 ++-
.../Algorithms/src/MonteCarloAbsorption.cpp | 7 +--
.../MCAbsorptionStrategy.cpp | 16 ++---
.../SampleCorrections/MCInteractionVolume.cpp | 62 ++++++++++---------
.../test/MCAbsorptionStrategyTest.h | 15 +++--
.../Algorithms/test/MCInteractionVolumeTest.h | 38 +++++++-----
7 files changed, 83 insertions(+), 69 deletions(-)
diff --git a/Framework/Algorithms/inc/MantidAlgorithms/SampleCorrections/MCAbsorptionStrategy.h b/Framework/Algorithms/inc/MantidAlgorithms/SampleCorrections/MCAbsorptionStrategy.h
index 47c0d2b6679..6c1cd2b3b12 100644
--- a/Framework/Algorithms/inc/MantidAlgorithms/SampleCorrections/MCAbsorptionStrategy.h
+++ b/Framework/Algorithms/inc/MantidAlgorithms/SampleCorrections/MCAbsorptionStrategy.h
@@ -18,6 +18,7 @@ class Sample;
namespace Kernel {
class PseudoRandomNumberGenerator;
class V3D;
+class Logger;
} // namespace Kernel
namespace Algorithms {
class IBeamProfile;
@@ -35,11 +36,10 @@ class MANTID_ALGORITHMS_DLL MCAbsorptionStrategy {
public:
MCAbsorptionStrategy(const IBeamProfile &beamProfile,
const API::Sample &sample, size_t nevents,
- size_t maxScatterPtAttempts);
+ size_t maxScatterPtAttempts, Kernel::Logger &logger);
std::tuple<double, double> calculate(Kernel::PseudoRandomNumberGenerator &rng,
const Kernel::V3D &finalPos,
- double lambdaBefore, double lambdaAfter,
- std::string &debugString);
+ double lambdaBefore, double lambdaAfter);
private:
const IBeamProfile &m_beamProfile;
diff --git a/Framework/Algorithms/inc/MantidAlgorithms/SampleCorrections/MCInteractionVolume.h b/Framework/Algorithms/inc/MantidAlgorithms/SampleCorrections/MCInteractionVolume.h
index 80eac35fec6..2e27737805d 100644
--- a/Framework/Algorithms/inc/MantidAlgorithms/SampleCorrections/MCInteractionVolume.h
+++ b/Framework/Algorithms/inc/MantidAlgorithms/SampleCorrections/MCInteractionVolume.h
@@ -9,6 +9,7 @@
#include "MantidAlgorithms/DllConfig.h"
#include "MantidGeometry/Objects/BoundingBox.h"
+#include "MantidKernel/Logger.h"
#include <boost/optional.hpp>
namespace Mantid {
@@ -36,18 +37,20 @@ class MANTID_ALGORITHMS_DLL MCInteractionVolume {
public:
MCInteractionVolume(const API::Sample &sample,
const Geometry::BoundingBox &activeRegion,
+ Kernel::Logger &logger,
const size_t maxScatterAttempts = 5000);
// No creation from temporaries as we store a reference to the object in
// the sample
MCInteractionVolume(const API::Sample &&sample,
- const Geometry::BoundingBox &&activeRegion) = delete;
+ const Geometry::BoundingBox &&activeRegion,
+ Kernel::Logger &logger) = delete;
const Geometry::BoundingBox &getBoundingBox() const;
double calculateAbsorption(Kernel::PseudoRandomNumberGenerator &rng,
const Kernel::V3D &startPos,
const Kernel::V3D &endPos, double lambdaBefore,
double lambdaAfter);
- std::string generateScatterPointStats() const;
+ void generateScatterPointStats();
Kernel::V3D generatePoint(Kernel::PseudoRandomNumberGenerator &rng);
private:
@@ -62,6 +65,7 @@ private:
const Geometry::SampleEnvironment *m_env;
const Geometry::BoundingBox m_activeRegion;
const size_t m_maxScatterAttempts;
+ Kernel::Logger &m_logger;
};
} // namespace Algorithms
diff --git a/Framework/Algorithms/src/MonteCarloAbsorption.cpp b/Framework/Algorithms/src/MonteCarloAbsorption.cpp
index abf3fefe620..ca8ac64e5e5 100644
--- a/Framework/Algorithms/src/MonteCarloAbsorption.cpp
+++ b/Framework/Algorithms/src/MonteCarloAbsorption.cpp
@@ -241,7 +241,7 @@ MatrixWorkspace_uptr MonteCarloAbsorption::doSimulation(
// Configure strategy
MCAbsorptionStrategy strategy(*beamProfile, inputWS.sample(), nevents,
- maxScatterPtAttempts);
+ maxScatterPtAttempts, g_log);
const auto &spectrumInfo = simulationWS.spectrumInfo();
@@ -276,12 +276,9 @@ MatrixWorkspace_uptr MonteCarloAbsorption::doSimulation(
} else {
// elastic case already initialized
}
- std::string debugString;
std::tie(outY[j], std::ignore) =
- strategy.calculate(rng, detPos, lambdaIn, lambdaOut, debugString);
-
- g_log.debug(debugString);
+ strategy.calculate(rng, detPos, lambdaIn, lambdaOut);
// Ensure we have the last point for the interpolation
if (lambdaStepSize > 1 && j + lambdaStepSize >= nbins && j + 1 != nbins) {
diff --git a/Framework/Algorithms/src/SampleCorrections/MCAbsorptionStrategy.cpp b/Framework/Algorithms/src/SampleCorrections/MCAbsorptionStrategy.cpp
index 3ce29eec39d..ecc657117f2 100644
--- a/Framework/Algorithms/src/SampleCorrections/MCAbsorptionStrategy.cpp
+++ b/Framework/Algorithms/src/SampleCorrections/MCAbsorptionStrategy.cpp
@@ -28,10 +28,11 @@ namespace Algorithms {
MCAbsorptionStrategy::MCAbsorptionStrategy(const IBeamProfile &beamProfile,
const API::Sample &sample,
size_t nevents,
- size_t maxScatterPtAttempts)
+ size_t maxScatterPtAttempts,
+ Kernel::Logger &logger)
: m_beamProfile(beamProfile),
- m_scatterVol(
- MCInteractionVolume(sample, beamProfile.defineActiveRegion(sample))),
+ m_scatterVol(MCInteractionVolume(
+ sample, beamProfile.defineActiveRegion(sample), logger)),
m_nevents(nevents), m_maxScatterAttempts(maxScatterPtAttempts),
m_error(1.0 / std::sqrt(m_nevents)) {}
@@ -46,9 +47,10 @@ MCAbsorptionStrategy::MCAbsorptionStrategy(const IBeamProfile &beamProfile,
* @param debugString String describing debug information from calculation
* @return A tuple of the <correction factor, associated error>.
*/
-std::tuple<double, double> MCAbsorptionStrategy::calculate(
- Kernel::PseudoRandomNumberGenerator &rng, const Kernel::V3D &finalPos,
- double lambdaBefore, double lambdaAfter, std::string &debugString) {
+std::tuple<double, double>
+MCAbsorptionStrategy::calculate(Kernel::PseudoRandomNumberGenerator &rng,
+ const Kernel::V3D &finalPos,
+ double lambdaBefore, double lambdaAfter) {
const auto scatterBounds = m_scatterVol.getBoundingBox();
double factor(0.0);
@@ -76,7 +78,7 @@ std::tuple<double, double> MCAbsorptionStrategy::calculate(
} while (true);
}
- debugString += m_scatterVol.generateScatterPointStats();
+ m_scatterVol.generateScatterPointStats();
using std::make_tuple;
return make_tuple(factor / static_cast<double>(m_nevents), m_error);
diff --git a/Framework/Algorithms/src/SampleCorrections/MCInteractionVolume.cpp b/Framework/Algorithms/src/SampleCorrections/MCInteractionVolume.cpp
index 47aef7b9ccd..71bdaf1274b 100644
--- a/Framework/Algorithms/src/SampleCorrections/MCInteractionVolume.cpp
+++ b/Framework/Algorithms/src/SampleCorrections/MCInteractionVolume.cpp
@@ -31,9 +31,10 @@ namespace Algorithms {
*/
MCInteractionVolume::MCInteractionVolume(
const API::Sample &sample, const Geometry::BoundingBox &activeRegion,
- const size_t maxScatterAttempts)
+ Kernel::Logger &logger, const size_t maxScatterAttempts)
: m_sample(sample.getShape().clone()), m_env(nullptr),
- m_activeRegion(activeRegion), m_maxScatterAttempts(maxScatterAttempts) {
+ m_activeRegion(activeRegion), m_maxScatterAttempts(maxScatterAttempts),
+ m_logger(logger) {
if (!m_sample->hasValidShape()) {
throw std::invalid_argument(
"MCInteractionVolume() - Sample shape does not have a valid shape.");
@@ -203,34 +204,37 @@ double MCInteractionVolume::calculateAbsorption(
* the simulated scatter points occurred in
* @return The generated string
*/
-std::string MCInteractionVolume::generateScatterPointStats() const {
- std::stringstream scatterPointSummary;
- scatterPointSummary << std::fixed;
- scatterPointSummary << std::setprecision(2);
-
- scatterPointSummary << "Scatter point counts:" << std::endl;
-
- int totalScatterPoints =
- std::accumulate(m_envScatterPoints.begin(), m_envScatterPoints.end(),
- m_sampleScatterPoints);
-
- scatterPointSummary << "Total scatter points: " << totalScatterPoints
- << std::endl;
-
- double percentage =
- static_cast<double>(m_sampleScatterPoints) / totalScatterPoints * 100;
- scatterPointSummary << "Sample: " << m_sampleScatterPoints << " ("
- << percentage << "%)" << std::endl;
-
- for (std::vector<int>::size_type i = 0; i < m_envScatterPoints.size(); i++) {
- percentage =
- static_cast<double>(m_envScatterPoints[i]) / totalScatterPoints * 100;
- scatterPointSummary << "Environment part " << i << " ("
- << m_env->getComponent(i).id()
- << "): " << m_envScatterPoints[i] << " (" << percentage
- << "%)" << std::endl;
+void MCInteractionVolume::generateScatterPointStats() {
+ if (m_logger.is(Kernel::Logger::Priority::PRIO_DEBUG)) {
+ std::stringstream scatterPointSummary;
+ scatterPointSummary << std::fixed;
+ scatterPointSummary << std::setprecision(2);
+
+ scatterPointSummary << "Scatter point counts:" << std::endl;
+
+ int totalScatterPoints =
+ std::accumulate(m_envScatterPoints.begin(), m_envScatterPoints.end(),
+ m_sampleScatterPoints);
+
+ scatterPointSummary << "Total scatter points: " << totalScatterPoints
+ << std::endl;
+
+ double percentage =
+ static_cast<double>(m_sampleScatterPoints) / totalScatterPoints * 100;
+ scatterPointSummary << "Sample: " << m_sampleScatterPoints << " ("
+ << percentage << "%)" << std::endl;
+
+ for (std::vector<int>::size_type i = 0; i < m_envScatterPoints.size();
+ i++) {
+ percentage =
+ static_cast<double>(m_envScatterPoints[i]) / totalScatterPoints * 100;
+ scatterPointSummary << "Environment part " << i << " ("
+ << m_env->getComponent(i).id()
+ << "): " << m_envScatterPoints[i] << " ("
+ << percentage << "%)" << std::endl;
+ }
+ m_logger.debug(scatterPointSummary.str());
}
- return scatterPointSummary.str();
}
} // namespace Algorithms
diff --git a/Framework/Algorithms/test/MCAbsorptionStrategyTest.h b/Framework/Algorithms/test/MCAbsorptionStrategyTest.h
index 31fcefa7323..3d763ee1140 100644
--- a/Framework/Algorithms/test/MCAbsorptionStrategyTest.h
+++ b/Framework/Algorithms/test/MCAbsorptionStrategyTest.h
@@ -13,6 +13,7 @@
#include "MantidAlgorithms/SampleCorrections/RectangularBeamProfile.h"
#include "MantidGeometry/Instrument/ReferenceFrame.h"
#include "MantidGeometry/Objects/BoundingBox.h"
+#include "MantidKernel/Logger.h"
#include "MantidKernel/WarningSuppressions.h"
#include "MonteCarloTesting.h"
@@ -42,7 +43,7 @@ public:
.WillOnce(Return(testSampleSphere.getShape().getBoundingBox()));
const size_t nevents(10), maxTries(100);
MCAbsorptionStrategy mcabsorb(testBeamProfile, testSampleSphere, nevents,
- maxTries);
+ maxTries, g_log);
// 3 random numbers per event expected
MockRNG rng;
EXPECT_CALL(rng, nextValue())
@@ -57,9 +58,8 @@ public:
const double lambdaBefore(2.5), lambdaAfter(3.5);
double factor(0.0), error(0.0);
- std::string debugString;
std::tie(factor, error) =
- mcabsorb.calculate(rng, endPos, lambdaBefore, lambdaAfter, debugString);
+ mcabsorb.calculate(rng, endPos, lambdaBefore, lambdaAfter);
TS_ASSERT_DELTA(0.0043828472, factor, 1e-08);
TS_ASSERT_DELTA(1.0 / std::sqrt(nevents), error, 1e-08);
}
@@ -81,15 +81,13 @@ public:
ReferenceFrame(Y, Z, Right, "source"), V3D(), 1, 1);
const size_t nevents(10), maxTries(1);
MCAbsorptionStrategy mcabs(testBeamProfile, testThinAnnulus, nevents,
- maxTries);
+ maxTries, g_log);
MockRNG rng;
EXPECT_CALL(rng, nextValue()).WillRepeatedly(Return(0.5));
const double lambdaBefore(2.5), lambdaAfter(3.5);
const V3D endPos(0.7, 0.7, 1.4);
- std::string debugString;
- TS_ASSERT_THROWS(
- mcabs.calculate(rng, endPos, lambdaBefore, lambdaAfter, debugString),
- const std::runtime_error &)
+ TS_ASSERT_THROWS(mcabs.calculate(rng, endPos, lambdaBefore, lambdaAfter),
+ const std::runtime_error &)
}
private:
@@ -106,6 +104,7 @@ private:
const Mantid::API::Sample &));
GNU_DIAG_ON_SUGGEST_OVERRIDE
};
+ Mantid::Kernel::Logger g_log{"MCAbsorptionStrategyTest"};
};
#endif /* MANTID_ALGORITHMS_MCABSORPTIONSTRATEGYTEST_H_ */
diff --git a/Framework/Algorithms/test/MCInteractionVolumeTest.h b/Framework/Algorithms/test/MCInteractionVolumeTest.h
index 5e0c057231c..0e652195fb2 100644
--- a/Framework/Algorithms/test/MCInteractionVolumeTest.h
+++ b/Framework/Algorithms/test/MCInteractionVolumeTest.h
@@ -10,6 +10,7 @@
#include <cxxtest/TestSuite.h>
#include "MantidAlgorithms/SampleCorrections/MCInteractionVolume.h"
+#include "MantidKernel/Logger.h"
#include "MantidKernel/MersenneTwister.h"
#include "MonteCarloTesting.h"
@@ -35,7 +36,7 @@ public:
void test_Bounding_Volume_Matches_Sample() {
auto sample = createTestSample(TestSampleType::SolidSphere);
const auto sampleBox = sample.getShape().getBoundingBox();
- MCInteractionVolume interactor(sample, sampleBox);
+ MCInteractionVolume interactor(sample, sampleBox, g_log);
const auto interactionBox = interactor.getBoundingBox();
TS_ASSERT_EQUALS(sampleBox.minPoint(), interactionBox.minPoint());
@@ -54,7 +55,8 @@ public:
.WillRepeatedly(Return(0.25));
auto sample = createTestSample(TestSampleType::SolidSphere);
- MCInteractionVolume interactor(sample, sample.getShape().getBoundingBox());
+ MCInteractionVolume interactor(sample, sample.getShape().getBoundingBox(),
+ g_log);
const double factor = interactor.calculateAbsorption(
rng, startPos, endPos, lambdaBefore, lambdaAfter);
TS_ASSERT_DELTA(0.0028357258, factor, 1e-8);
@@ -74,7 +76,8 @@ public:
.Times(Exactly(3))
.WillRepeatedly(Return(0.25));
- MCInteractionVolume interactor(sample, sample.getShape().getBoundingBox());
+ MCInteractionVolume interactor(sample, sample.getShape().getBoundingBox(),
+ g_log);
const double factorSeg1 = interactor.calculateAbsorption(
rng, startPos, endPos, lambdaBefore, lambdaAfter);
TS_ASSERT_DELTA(0.030489479, factorSeg1, 1e-8);
@@ -110,8 +113,8 @@ public:
.WillOnce(Return(0.5)) // r2
.WillOnce(Return(0.5)); // r3
- MCInteractionVolume interactor(sample,
- sample.getEnvironment().boundingBox());
+ MCInteractionVolume interactor(
+ sample, sample.getEnvironment().boundingBox(), g_log);
const double factorContainer = interactor.calculateAbsorption(
rng, startPos, endPos, lambdaBefore, lambdaAfter);
TS_ASSERT_DELTA(0.69223681, factorContainer, 1e-8);
@@ -136,13 +139,13 @@ public:
void test_Construction_With_Invalid_Sample_Shape_Throws_Error() {
Mantid::API::Sample sample;
// nothing
- TS_ASSERT_THROWS(
- MCInteractionVolume mcv(sample, sample.getShape().getBoundingBox()),
- const std::invalid_argument &);
+ TS_ASSERT_THROWS(MCInteractionVolume mcv(
+ sample, sample.getShape().getBoundingBox(), g_log),
+ const std::invalid_argument &);
// valid shape
sample.setShape(ComponentCreationHelper::createSphere(1));
- TS_ASSERT_THROWS_NOTHING(
- MCInteractionVolume mcv(sample, sample.getShape().getBoundingBox()));
+ TS_ASSERT_THROWS_NOTHING(MCInteractionVolume mcv(
+ sample, sample.getShape().getBoundingBox(), g_log));
}
void test_Throws_If_Point_Cannot_Be_Generated() {
@@ -155,8 +158,9 @@ public:
MersenneTwister rng;
rng.setSeed(1);
const size_t maxTries(1);
+ Mantid::Kernel::Logger g_log("MCInteractionVolumeTest");
MCInteractionVolume interactor(sample, sample.getShape().getBoundingBox(),
- maxTries);
+ g_log, maxTries);
TS_ASSERT_THROWS(interactor.calculateAbsorption(rng, startPos, endPos,
lambdaBefore, lambdaAfter),
const std::runtime_error &);
@@ -172,8 +176,9 @@ public:
sample.setEnvironment(
std::make_unique<Mantid::Geometry::SampleEnvironment>(*kit));
+ Mantid::Kernel::Logger g_log("MCInteractionVolumeTest");
MCInteractionVolume interactor(
- sample, kit->getComponent(0).getBoundingBox(), maxAttempts);
+ sample, kit->getComponent(0).getBoundingBox(), g_log, maxAttempts);
// Generate "random" sequence
MockRNG rng;
@@ -191,7 +196,7 @@ public:
// Selects second component
MCInteractionVolume interactor2(
- sample, kit->getComponent(1).getBoundingBox(), maxAttempts);
+ sample, kit->getComponent(1).getBoundingBox(), g_log, maxAttempts);
EXPECT_CALL(rng, nextInt(_, _)).Times(Exactly(1)).WillOnce(Return(2));
EXPECT_CALL(rng, nextValue())
@@ -207,7 +212,7 @@ public:
// Selects third component
MCInteractionVolume interactor3(
- sample, kit->getComponent(2).getBoundingBox(), maxAttempts);
+ sample, kit->getComponent(2).getBoundingBox(), g_log, maxAttempts);
EXPECT_CALL(rng, nextInt(_, _)).Times(Exactly(1)).WillOnce(Return(3));
EXPECT_CALL(rng, nextValue())
.Times(3)
@@ -240,11 +245,14 @@ public:
std::make_unique<Mantid::Geometry::SampleEnvironment>(*kit));
MCInteractionVolume interactor(sample, kit->getContainer().getBoundingBox(),
- maxAttempts);
+ g_log, maxAttempts);
// Restrict region to can
TS_ASSERT_THROWS(interactor.generatePoint(rng), const std::runtime_error &);
Mock::VerifyAndClearExpectations(&rng);
}
+
+private:
+ Mantid::Kernel::Logger g_log{"MCInteractionVolumeTest"};
};
#endif /* MANTID_ALGORITHMS_MCINTERACTIONVOLUMETEST_H_ */
--
GitLab