From 8096459381efaa5b531b118ede4545795d0e0888 Mon Sep 17 00:00:00 2001 From: Tom Hampson Date: Tue, 5 Mar 2024 11:44:53 +0000 Subject: [PATCH 1/6] Add test exposing CompositeFunction bug --- Framework/API/test/CompositeFunctionTest.h | 35 ++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/Framework/API/test/CompositeFunctionTest.h b/Framework/API/test/CompositeFunctionTest.h index 6a41b82868b2..b61883bcc02c 100644 --- a/Framework/API/test/CompositeFunctionTest.h +++ b/Framework/API/test/CompositeFunctionTest.h @@ -139,6 +139,28 @@ template class Gauss : public IPeakFunction { void setFwhm(const double w) override { setParameter(2, w); } }; +// Create class with configurable number of domains for ease of testing. +class FunctionWithNDomains : public IPeakFunction { +public: + FunctionWithNDomains(int nDomains) : m_nDomains(nDomains) {} + + size_t getNumberDomains() const override { return m_nDomains; } + + // Override pure virtual functions. + std::string name() const override { return "FunctionWithNDomains"; } + + double fwhm() const override { return 1; } + void setFwhm(const double w) override {} + void functionLocal(double *out, const double *xValues, const size_t nData) const override {} + double centre() const override { return 1; } + double height() const override { return 1; } + void setCentre(const double c) override {} + void setHeight(const double h) override {} + +private: + size_t m_nDomains; +}; + template class Linear : public ParamFunction, public IFunction1D { public: Linear() { @@ -1517,6 +1539,19 @@ class CompositeFunctionTest : public CxxTest::TestSuite { TS_ASSERT_EQUALS(composite->getFunction(1)->calculateStepSize(parameterValue2), parameterValue2 * sqrtEpsilon); } + void test_getNumberDomains_throws_with_inconsistent_domain_numbers() { + IFunction_sptr f1 = IFunction_sptr(new FunctionWithNDomains(7)); + IFunction_sptr f2 = IFunction_sptr(new FunctionWithNDomains(13)); + + CompositeFunction fun; + + fun.addFunction(f1); + fun.addFunction(f2); + + TS_ASSERT_THROWS_EQUALS(fun.getNumberDomains(), const std::runtime_error &e, std::string(e.what()), + "CompositeFunction has members with inconsistent domain numbers."); + } + private: CompositeFunction_sptr createComposite() { auto composite = std::make_unique(); From b33ca4101c48ed5fb08cb4a40c540ea3d066011c Mon Sep 17 00:00:00 2001 From: Tom Hampson Date: Tue, 5 Mar 2024 12:04:26 +0000 Subject: [PATCH 2/6] Fix indexing bug --- Framework/API/src/CompositeFunction.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Framework/API/src/CompositeFunction.cpp b/Framework/API/src/CompositeFunction.cpp index 1edabbd44769..bba42b4c9d90 100644 --- a/Framework/API/src/CompositeFunction.cpp +++ b/Framework/API/src/CompositeFunction.cpp @@ -965,7 +965,7 @@ size_t CompositeFunction::getNumberDomains() const { } size_t nd = getFunction(0)->getNumberDomains(); for (size_t iFun = 1; iFun < n; ++iFun) { - if (getFunction(0)->getNumberDomains() != nd) { + if (getFunction(iFun)->getNumberDomains() != nd) { throw std::runtime_error("CompositeFunction has members with " "inconsistent domain numbers."); } From 391e301640bfa7b326f10568f3d71a621829c5c6 Mon Sep 17 00:00:00 2001 From: Tom Hampson Date: Tue, 5 Mar 2024 12:10:46 +0000 Subject: [PATCH 3/6] Remove cppcheck suppression --- buildconfig/CMake/CppCheck_Suppressions.txt.in | 1 - 1 file changed, 1 deletion(-) diff --git a/buildconfig/CMake/CppCheck_Suppressions.txt.in b/buildconfig/CMake/CppCheck_Suppressions.txt.in index 1e28adca9561..9ea13a3000f3 100644 --- a/buildconfig/CMake/CppCheck_Suppressions.txt.in +++ b/buildconfig/CMake/CppCheck_Suppressions.txt.in @@ -321,7 +321,6 @@ constParameterPointer:${CMAKE_SOURCE_DIR}/Framework/API/src/BoostOptionalToAlgor constVariablePointer:${CMAKE_SOURCE_DIR}/Framework/API/src/BoostOptionalToAlgorithmProperty.cpp:33 constParameterPointer:${CMAKE_SOURCE_DIR}/Framework/API/src/BoostOptionalToAlgorithmProperty.cpp:30 unreadVariable:${CMAKE_SOURCE_DIR}/Framework/Algorithms/src/SmoothNeighbours.cpp:317 -knownConditionTrueFalse:${CMAKE_SOURCE_DIR}/Framework/API/src/CompositeFunction.cpp:968 constVariablePointer:${CMAKE_SOURCE_DIR}/Framework/API/src/CompositeFunction.cpp:560 constVariablePointer:${CMAKE_SOURCE_DIR}/Framework/API/src/CompositeFunction.cpp:879 constVariablePointer:${CMAKE_SOURCE_DIR}/Framework/API/src/CompositeFunction.cpp:906 From c001843c4a76ea217b6da15f967b21a73bb68a49 Mon Sep 17 00:00:00 2001 From: Tom Hampson Date: Tue, 5 Mar 2024 16:21:54 +0000 Subject: [PATCH 4/6] Fix compiler warnings --- Framework/API/test/CompositeFunctionTest.h | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/Framework/API/test/CompositeFunctionTest.h b/Framework/API/test/CompositeFunctionTest.h index b61883bcc02c..1dde816314f0 100644 --- a/Framework/API/test/CompositeFunctionTest.h +++ b/Framework/API/test/CompositeFunctionTest.h @@ -148,14 +148,17 @@ class FunctionWithNDomains : public IPeakFunction { // Override pure virtual functions. std::string name() const override { return "FunctionWithNDomains"; } - double fwhm() const override { return 1; } - void setFwhm(const double w) override {} - void functionLocal(double *out, const double *xValues, const size_t nData) const override {} + void setFwhm(const double w) override { UNUSED_ARG(w); } + void functionLocal(double *out, const double *xValues, const size_t nData) const override { + UNUSED_ARG(out); + UNUSED_ARG(xValues); + UNUSED_ARG(nData); + } double centre() const override { return 1; } double height() const override { return 1; } - void setCentre(const double c) override {} - void setHeight(const double h) override {} + void setCentre(const double c) override { UNUSED_ARG(c); } + void setHeight(const double h) override { UNUSED_ARG(h); } private: size_t m_nDomains; From cd7d75fe05212240f75c99759da89551dbcd8f78 Mon Sep 17 00:00:00 2001 From: Tom Hampson Date: Wed, 6 Mar 2024 10:35:11 +0000 Subject: [PATCH 5/6] Add release notes --- .../release/v6.10.0/Framework/Fit_Functions/Bugfixes/36970.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 docs/source/release/v6.10.0/Framework/Fit_Functions/Bugfixes/36970.rst diff --git a/docs/source/release/v6.10.0/Framework/Fit_Functions/Bugfixes/36970.rst b/docs/source/release/v6.10.0/Framework/Fit_Functions/Bugfixes/36970.rst new file mode 100644 index 000000000000..b1b3957f0c02 --- /dev/null +++ b/docs/source/release/v6.10.0/Framework/Fit_Functions/Bugfixes/36970.rst @@ -0,0 +1 @@ +- CompositeFunction will now throw an exception if getNumberDomains() is called and there is an inconsistent number of domains in any of the member functions. \ No newline at end of file From e4105f2f2c24b5bf7a8a426553aca73c9710eb3d Mon Sep 17 00:00:00 2001 From: Tom Hampson Date: Wed, 6 Mar 2024 11:08:40 +0000 Subject: [PATCH 6/6] fix typo in function name --- Framework/API/test/CompositeFunctionTest.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Framework/API/test/CompositeFunctionTest.h b/Framework/API/test/CompositeFunctionTest.h index 1dde816314f0..4645b4797315 100644 --- a/Framework/API/test/CompositeFunctionTest.h +++ b/Framework/API/test/CompositeFunctionTest.h @@ -1359,7 +1359,7 @@ class CompositeFunctionTest : public CxxTest::TestSuite { TS_ASSERT_EQUALS(mfun->getAttribute("NumDeriv").asBool(), true); } - void test_set_attribute_thorws_if_attribute_not_recongized() { + void test_set_attribute_throws_if_attribute_not_recongized() { auto mfun = std::make_unique(); auto gauss = std::make_shared>(); auto background = std::make_shared>();