From 03e63ef52b070bf75a717968e3676790f47c07d7 Mon Sep 17 00:00:00 2001
From: Danny Hindson <danny.hindson@stfc.ac.uk>
Date: Thu, 23 Jan 2020 08:22:53 +0000
Subject: [PATCH] Fix seg fault in Jenkins tests
The access to the debugstring member of MCInteractionVolume wasn't thread safe
Changed to be a string parameter passed in per thread
---
.../SampleCorrections/MCAbsorptionStrategy.h | 5 ++---
Framework/Algorithms/src/MonteCarloAbsorption.cpp | 6 ++++--
.../src/SampleCorrections/MCAbsorptionStrategy.cpp | 9 +++++----
.../src/SampleCorrections/MCInteractionVolume.cpp | 3 ++-
Framework/Algorithms/test/MCAbsorptionStrategyTest.h | 9 ++++++---
5 files changed, 19 insertions(+), 13 deletions(-)
diff --git a/Framework/Algorithms/inc/MantidAlgorithms/SampleCorrections/MCAbsorptionStrategy.h b/Framework/Algorithms/inc/MantidAlgorithms/SampleCorrections/MCAbsorptionStrategy.h
index fd4d706b57c..47c0d2b6679 100644
--- a/Framework/Algorithms/inc/MantidAlgorithms/SampleCorrections/MCAbsorptionStrategy.h
+++ b/Framework/Algorithms/inc/MantidAlgorithms/SampleCorrections/MCAbsorptionStrategy.h
@@ -38,8 +38,8 @@ public:
size_t maxScatterPtAttempts);
std::tuple<double, double> calculate(Kernel::PseudoRandomNumberGenerator &rng,
const Kernel::V3D &finalPos,
- double lambdaBefore, double lambdaAfter);
- std::string getDebugString() const { return debugString; };
+ double lambdaBefore, double lambdaAfter,
+ std::string &debugString);
private:
const IBeamProfile &m_beamProfile;
@@ -47,7 +47,6 @@ private:
const size_t m_nevents;
const size_t m_maxScatterAttempts;
const double m_error;
- std::string debugString;
};
} // namespace Algorithms
diff --git a/Framework/Algorithms/src/MonteCarloAbsorption.cpp b/Framework/Algorithms/src/MonteCarloAbsorption.cpp
index 83d75020a49..abf3fefe620 100644
--- a/Framework/Algorithms/src/MonteCarloAbsorption.cpp
+++ b/Framework/Algorithms/src/MonteCarloAbsorption.cpp
@@ -276,10 +276,12 @@ MatrixWorkspace_uptr MonteCarloAbsorption::doSimulation(
} else {
// elastic case already initialized
}
+ std::string debugString;
std::tie(outY[j], std::ignore) =
- strategy.calculate(rng, detPos, lambdaIn, lambdaOut);
- g_log.debug(strategy.getDebugString());
+ strategy.calculate(rng, detPos, lambdaIn, lambdaOut, debugString);
+
+ g_log.debug(debugString);
// 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 40f44015131..3ce29eec39d 100644
--- a/Framework/Algorithms/src/SampleCorrections/MCAbsorptionStrategy.cpp
+++ b/Framework/Algorithms/src/SampleCorrections/MCAbsorptionStrategy.cpp
@@ -43,14 +43,15 @@ MCAbsorptionStrategy::MCAbsorptionStrategy(const IBeamProfile &beamProfile,
* where it is detected
* @param lambdaBefore Wavelength, in \f$\\A^-1\f$, before scattering
* @param lambdaAfter Wavelength, in \f$\\A^-1\f$, after scattering
+ * @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::tuple<double, double> MCAbsorptionStrategy::calculate(
+ Kernel::PseudoRandomNumberGenerator &rng, const Kernel::V3D &finalPos,
+ double lambdaBefore, double lambdaAfter, std::string &debugString) {
const auto scatterBounds = m_scatterVol.getBoundingBox();
double factor(0.0);
+
for (size_t i = 0; i < m_nevents; ++i) {
size_t attempts(0);
do {
diff --git a/Framework/Algorithms/src/SampleCorrections/MCInteractionVolume.cpp b/Framework/Algorithms/src/SampleCorrections/MCInteractionVolume.cpp
index cc85412265e..47aef7b9ccd 100644
--- a/Framework/Algorithms/src/SampleCorrections/MCInteractionVolume.cpp
+++ b/Framework/Algorithms/src/SampleCorrections/MCInteractionVolume.cpp
@@ -91,7 +91,7 @@ int MCInteractionVolume::getComponentIndex(
boost::optional<Kernel::V3D> MCInteractionVolume::generatePointInObjectByIndex(
int componentIndex, Kernel::PseudoRandomNumberGenerator &rng) {
- boost::optional<Kernel::V3D> pointGenerated;
+ boost::optional<Kernel::V3D> pointGenerated{boost::none};
if (componentIndex == -1) {
pointGenerated = m_sample->generatePointInObject(rng, m_activeRegion, 1);
} else {
@@ -161,6 +161,7 @@ double MCInteractionVolume::calculateAbsorption(
// is calculated in reverse, i.e. defining the track from the scatter pt
// backwards for simplicity with how the Track object works. This avoids
// having to understand exactly which object the scattering occurred in.
+
V3D scatterPos = generatePoint(rng);
const auto toStart = normalize(startPos - scatterPos);
diff --git a/Framework/Algorithms/test/MCAbsorptionStrategyTest.h b/Framework/Algorithms/test/MCAbsorptionStrategyTest.h
index c23ed83a607..31fcefa7323 100644
--- a/Framework/Algorithms/test/MCAbsorptionStrategyTest.h
+++ b/Framework/Algorithms/test/MCAbsorptionStrategyTest.h
@@ -57,8 +57,9 @@ 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);
+ mcabsorb.calculate(rng, endPos, lambdaBefore, lambdaAfter, debugString);
TS_ASSERT_DELTA(0.0043828472, factor, 1e-08);
TS_ASSERT_DELTA(1.0 / std::sqrt(nevents), error, 1e-08);
}
@@ -85,8 +86,10 @@ public:
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);
- TS_ASSERT_THROWS(mcabs.calculate(rng, endPos, lambdaBefore, lambdaAfter),
- const std::runtime_error &)
+ std::string debugString;
+ TS_ASSERT_THROWS(
+ mcabs.calculate(rng, endPos, lambdaBefore, lambdaAfter, debugString),
+ const std::runtime_error &)
}
private:
--
GitLab