From 61fccfb27d064b8aef0e0a86b1ca81b9a311e385 Mon Sep 17 00:00:00 2001 From: Matthias Puchner <github@mpuchner.de> Date: Tue, 4 May 2021 15:09:11 +0200 Subject: [PATCH] doku, add unit tests, minor refactorings --- Device/Beam/Beam.cpp | 26 +++++-------------- Device/Beam/Beam.h | 15 ++++++----- Param/Base/IParametricComponent.h | 13 +++++++--- Param/Distrib/Distributions.h | 9 ------- Sample/Multilayer/MultiLayer.cpp | 8 +++--- Sample/Multilayer/MultiLayer.h | 4 +-- .../Core/Parameters/IParameterizedTest.cpp | 19 +++++++++++++- 7 files changed, 48 insertions(+), 46 deletions(-) diff --git a/Device/Beam/Beam.cpp b/Device/Beam/Beam.cpp index b3a3e9400f6..9903eb5a1c2 100644 --- a/Device/Beam/Beam.cpp +++ b/Device/Beam/Beam.cpp @@ -19,23 +19,6 @@ #include "Param/Base/RealParameter.h" -namespace { -void setGuarded(double* p, const RealLimits& limits, const std::string& paramName, double value) -{ - if (value == *p) - return; // nothing to do - - if (!limits.isInRange(value)) { - std::ostringstream message; - message << "Cannot set " << paramName << " to value " << value << ": out of bounds [" - << limits << "]\n"; - throw std::runtime_error(message.str()); - } - - *p = value; -} -} // namespace - // Allow for 90 degrees by adding a relatively small constant to pi/2 static constexpr double INCLINATION_LIMIT = M_PI_2 + 1e-10; @@ -133,17 +116,20 @@ const IFootprintFactor* Beam::footprintFactor() const void Beam::setInclinationAngleGuarded(double value) { - setGuarded(&m_alpha, m_alphaLimits, "inclination angle", value); + checkLimits("inclination angle", value, m_alphaLimits); + m_alpha = value; } void Beam::setAzimuthalAngleGuarded(double value) { - setGuarded(&m_phi, m_phiLimits, "azimuthal angle", value); + checkLimits("azimuthal angle", value, m_phiLimits); + m_phi = value; } void Beam::setWavelengthGuarded(double value) { - setGuarded(&m_wavelength, m_wavelengthLimits, "wavelength", value); + checkLimits("wavelength", value, m_wavelengthLimits); + m_wavelength = value; } void Beam::setFootprintFactor(const IFootprintFactor& shape_factor) diff --git a/Device/Beam/Beam.h b/Device/Beam/Beam.h index 016d2a44d0a..f8769a29aa8 100644 --- a/Device/Beam/Beam.h +++ b/Device/Beam/Beam.h @@ -51,14 +51,17 @@ public: //! Returns footprint factor. const IFootprintFactor* footprintFactor() const; - //! set the value and check its limits - void setInclinationAngleGuarded(double d); // #baPool + add unit tests + //! Check for limits, set the value within limits. + //! Throws if limits are violated. + void setInclinationAngleGuarded(double d); - //! set the value and check its limits - void setAzimuthalAngleGuarded(double d); // #baPool + add unit tests + //! Check for limits, set the value within limits. + //! Throws if limits are violated. + void setAzimuthalAngleGuarded(double d); - //! set the value and check its limits - void setWavelengthGuarded(double d); // #baPool + add unit tests + //! Check for limits, set the value within limits. + //! Throws if limits are violated. + void setWavelengthGuarded(double d); #ifndef SWIG //! Returns the polarization density matrix (in spin basis along z-axis) diff --git a/Param/Base/IParametricComponent.h b/Param/Base/IParametricComponent.h index 6e06d23df71..a1be79a761b 100644 --- a/Param/Base/IParametricComponent.h +++ b/Param/Base/IParametricComponent.h @@ -25,7 +25,8 @@ class RealLimits; class ParameterPool; class RealParameter; -//! Manages a local parameter pool, and a tree of child pools. +//! Originally managed registered parameters. Is somehow a left-over of the big refactoring when +//! these parameters have been taken out. Maybe could be refactored to be put into IComponent. //! @ingroup tools_internal class IParametricComponent : public IComponent { @@ -38,14 +39,18 @@ public: const std::string& getName() const { return m_name; } - //! Action to be taken in inherited class when a parameter has changed. + //! Action to be taken in inherited class when a value has changed. Formerly this was also + //! called when a change via registered parameter took place. This is now obsolete, but the + //! onChange() call is still relevant for calculations when a value is changed by different + //! means (e.g. after initialization). virtual void onChange() {} - protected: void setName(const std::string& name) { m_name = name; } - // #bapool add docu and unit tests + //! Check a certain value against limits. + //! Throws if not within the limits. + //! Name is only for exception message. void checkLimits(const std::string& name, const double value, const RealLimits& limits) const; private: diff --git a/Param/Distrib/Distributions.h b/Param/Distrib/Distributions.h index 7ba4a422255..75e3ddabfa9 100644 --- a/Param/Distrib/Distributions.h +++ b/Param/Distrib/Distributions.h @@ -76,15 +76,6 @@ protected: //! Returns weighted samples from given interpolation points and probabilityDensity(). std::vector<ParameterSample> generateSamplesFromValues(const std::vector<double>& sample_values) const; - - friend class DistributionsTest_DistributionGateParameters_Test; // #baPool o remove; for - // setUnits - friend class DistributionsTest_DistributionLorentzParameters_Test; // #baPool o remove; for - // setUnits - friend class DistributionsTest_DistributionGaussianParameters_Test; // #baPool o remove; for - // setUnits - friend class DistributionsTest_DistributionCosineParameters_Test; // #baPool o remove; for - // setUnits }; #endif // USER_API diff --git a/Sample/Multilayer/MultiLayer.cpp b/Sample/Multilayer/MultiLayer.cpp index 3ab424801cd..56ac00d500e 100644 --- a/Sample/Multilayer/MultiLayer.cpp +++ b/Sample/Multilayer/MultiLayer.cpp @@ -66,7 +66,7 @@ void MultiLayer::addLayerWithTopRoughness(const Layer& layer, const LayerRoughne interface = LayerInterface::createRoughInterface(last_layer, new_layer, roughness); else interface = LayerInterface::createSmoothInterface(last_layer, new_layer); - addAndRegisterInterface(interface); + addInterface(interface); } else { // the top layer if (new_layer->thickness() != 0.0) { @@ -83,7 +83,7 @@ void MultiLayer::addLayerWithTopRoughness(const Layer& layer, const LayerRoughne "cannot have roughness."); } } - addAndRegisterLayer(new_layer); + addLayer(new_layer); } const Layer* MultiLayer::layer(size_t i_layer) const @@ -128,13 +128,13 @@ std::vector<const INode*> MultiLayer::getChildren() const return ret; } -void MultiLayer::addAndRegisterLayer(Layer* child) // #baPool + rename to addLayer +void MultiLayer::addLayer(Layer* child) { m_layers.push_back(child); registerChild(child); } -void MultiLayer::addAndRegisterInterface(LayerInterface* child) // #baPool + rename to addInterface +void MultiLayer::addInterface(LayerInterface* child) { m_interfaces.push_back(child); registerChild(child); diff --git a/Sample/Multilayer/MultiLayer.h b/Sample/Multilayer/MultiLayer.h index fc534c3457f..596e28d70a6 100644 --- a/Sample/Multilayer/MultiLayer.h +++ b/Sample/Multilayer/MultiLayer.h @@ -73,10 +73,10 @@ public: private: //! Adds the layer with simultaneous registration in parent class - void addAndRegisterLayer(Layer* child); + void addLayer(Layer* child); //! Adds the interface with simultaneous registration in parent class - void addAndRegisterInterface(LayerInterface* child); + void addInterface(LayerInterface* child); //! Checks index of layer w.r.t. vector length size_t check_layer_index(size_t i_layer) const; diff --git a/Tests/UnitTests/Core/Parameters/IParameterizedTest.cpp b/Tests/UnitTests/Core/Parameters/IParameterizedTest.cpp index c7fe76a2cb7..8c81fbbef3e 100644 --- a/Tests/UnitTests/Core/Parameters/IParameterizedTest.cpp +++ b/Tests/UnitTests/Core/Parameters/IParameterizedTest.cpp @@ -1,5 +1,22 @@ +#include "Fit/Param/RealLimits.h" #include "Param/Base/IParametricComponent.h" -#include "Param/Base/RealParameter.h" #include "Tests/GTestWrapper/google_test.h" #include <stdexcept> +class IParametricComponentTest : public ::testing::Test { +protected: + class TestClass : public IParametricComponent { + public: + using IParametricComponent::checkLimits; // make it public + }; +}; + +TEST_F(IParametricComponentTest, checkLimits) +{ + // this checks only the function if IParametricComponentTest::checkLimits() in principle. More + // tests regarding RealLimits shall be done in dedicated RealLimits unit tests + TestClass t; + EXPECT_NO_THROW(t.checkLimits("A", 0, RealLimits::nonnegative())); + EXPECT_THROW(t.checkLimits("A", 0, RealLimits::positive()), std::runtime_error); + EXPECT_NO_THROW(t.checkLimits("A", -1, RealLimits::limitless())); +} -- GitLab