Incorporate review comment relating to debug logging

Logging stats are only generated now if the logging level is debug

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;

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

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

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

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

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_ */

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_ */