From d00738d97c6b3c86a818641a0d94d343ce162d06 Mon Sep 17 00:00:00 2001 From: Maxim Smolskiy Date: Wed, 21 May 2025 08:36:35 +0300 Subject: [PATCH] Merge pull request #27331 from MaximSmolskiy:add-test-for-solveCubic Add tests for solveCubic #27331 ### Pull Request Readiness Checklist Related to #27323 I found only randomized tests with number of roots always equal to `1` or `3`, `x^3 = 0` and some simple test for Java and Swift. Obviously, they don't cover all cases (implementation has strong branching and number of roots can be equal to `-1`, `0` and `2` additionally). So, I think it will be useful to try explicitly cover more cases (and implementation branches correspondingly) See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request - [x] I agree to contribute to the project under Apache 2 License. - [x] To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV - [x] The PR is proposed to the proper branch - [x] There is a reference to the original bug report and related work - [x] There is accuracy test, performance test and test data in opencv_extra repository, if applicable Patch to opencv_extra has the same branch name. - [x] The feature is well documented and sample code can be built with the project CMake --- modules/core/include/opencv2/core.hpp | 4 +- modules/core/src/mathfuncs.cpp | 3 +- modules/core/test/test_math.cpp | 152 ++++++++++++++++++++++++++ 3 files changed, 156 insertions(+), 3 deletions(-) diff --git a/modules/core/include/opencv2/core.hpp b/modules/core/include/opencv2/core.hpp index d134273753..12951d2a98 100644 --- a/modules/core/include/opencv2/core.hpp +++ b/modules/core/include/opencv2/core.hpp @@ -2004,8 +2004,8 @@ The function solveCubic finds the real roots of a cubic equation: The roots are stored in the roots array. @param coeffs equation coefficients, an array of 3 or 4 elements. -@param roots output array of real roots that has 1 or 3 elements. -@return number of real roots. It can be 0, 1 or 2. +@param roots output array of real roots that has 0, 1, 2 or 3 elements. +@return number of real roots. It can be -1 (all real numbers), 0, 1, 2 or 3. */ CV_EXPORTS_W int solveCubic(InputArray coeffs, OutputArray roots); diff --git a/modules/core/src/mathfuncs.cpp b/modules/core/src/mathfuncs.cpp index ac037e38c7..7af66c5723 100644 --- a/modules/core/src/mathfuncs.cpp +++ b/modules/core/src/mathfuncs.cpp @@ -1590,7 +1590,7 @@ int cv::solveCubic( InputArray _coeffs, OutputArray _roots ) { if( a1 == 0 ) { - if( a2 == 0 ) + if( a2 == 0 ) // constant n = a3 == 0 ? -1 : 0; else { @@ -1624,6 +1624,7 @@ int cv::solveCubic( InputArray _coeffs, OutputArray _roots ) } else { + // cubic equation a0 = 1./a0; a1 *= a0; a2 *= a0; diff --git a/modules/core/test/test_math.cpp b/modules/core/test/test_math.cpp index b1777a7752..dc743dcbc1 100644 --- a/modules/core/test/test_math.cpp +++ b/modules/core/test/test_math.cpp @@ -2445,6 +2445,158 @@ static void checkRoot(Mat& r, T re, T im) } GTEST_NONFATAL_FAILURE_("Can't find root") << "(" << re << ", " << im << ")"; } + +TEST(Core_SolveCubicConstant, accuracy) +{ + { + const std::vector coeffs{0., 0., 0., 1.}; + std::vector roots; + const auto num_roots = solveCubic(coeffs, roots); + + EXPECT_EQ(num_roots, 0); + } + + { + const std::vector coeffs{0., 0., 0., 0.}; + std::vector roots; + const auto num_roots = solveCubic(coeffs, roots); + + EXPECT_EQ(num_roots, -1); + } +} + +TEST(Core_SolveCubicLinear, accuracy) +{ + const std::vector coeffs{0., 0., 2., -2.}; + std::vector roots; + const auto num_roots = solveCubic(coeffs, roots); + + EXPECT_EQ(num_roots, 1); + EXPECT_EQ(roots[0], 1.); +} + +TEST(Core_SolveCubicQuadratic, accuracy) +{ + { + const std::vector coeffs{0., 2., -4., 4.}; + std::vector roots; + const auto num_roots = solveCubic(coeffs, roots); + + EXPECT_EQ(num_roots, 0); + } + + { + const std::vector coeffs{0., 2., -4., 2.}; + std::vector roots; + const auto num_roots = solveCubic(coeffs, roots); + + EXPECT_EQ(num_roots, 1); + EXPECT_EQ(roots[0], 1.); + } + + { + const std::vector coeffs{0., 2., -6., 4.}; + std::vector roots; + const auto num_roots = solveCubic(coeffs, roots); + + EXPECT_EQ(num_roots, 2); + EXPECT_EQ(roots[0], 2.); + EXPECT_EQ(roots[1], 1.); + } +} + +TEST(Core_SolveCubicCubic, accuracy) +{ + { + const std::vector coeffs{2., -6., 6., -2.}; + std::vector roots; + const auto num_roots = solveCubic(coeffs, roots); + + EXPECT_EQ(num_roots, 1); + EXPECT_EQ(roots[0], 1.); + } + + { + const std::vector coeffs{2., -10., 24., -16.}; + std::vector roots; + const auto num_roots = solveCubic(coeffs, roots); + + EXPECT_EQ(num_roots, 1); + EXPECT_EQ(roots[0], 1.); + } + + { + const std::vector coeffs{2., -10., 16., -8.}; + std::vector roots; + const auto num_roots = solveCubic(coeffs, roots); + + EXPECT_TRUE(num_roots == 2 || num_roots == 3); + EXPECT_NEAR(roots[0], 1., 1e-8); + EXPECT_NEAR(roots[1], 2., 1e-8); + if (num_roots == 3) + { + EXPECT_NEAR(roots[2], 2., 1e-8); + } + } + + { + const std::vector coeffs{2., -12., 22., -12.}; + std::vector roots; + const auto num_roots = solveCubic(coeffs, roots); + + EXPECT_EQ(num_roots, 3); + EXPECT_NEAR(roots[0], 1., 1e-8); + EXPECT_NEAR(roots[1], 3., 1e-8); + EXPECT_NEAR(roots[2], 2., 1e-8); + } +} + +TEST(Core_SolveCubicNormalizedCubic, accuracy) +{ + { + const std::vector coeffs{-3., 3., -1.}; + std::vector roots; + const auto num_roots = solveCubic(coeffs, roots); + + EXPECT_EQ(num_roots, 1); + EXPECT_EQ(roots[0], 1.); + } + + { + const std::vector coeffs{-5., 12., -8.}; + std::vector roots; + const auto num_roots = solveCubic(coeffs, roots); + + EXPECT_EQ(num_roots, 1); + EXPECT_EQ(roots[0], 1.); + } + + { + const std::vector coeffs{-5., 8., -4.}; + std::vector roots; + const auto num_roots = solveCubic(coeffs, roots); + + EXPECT_TRUE(num_roots == 2 || num_roots == 3); + EXPECT_NEAR(roots[0], 1., 1e-8); + EXPECT_NEAR(roots[1], 2., 1e-8); + if (num_roots == 3) + { + EXPECT_NEAR(roots[2], 2., 1e-8); + } + } + + { + const std::vector coeffs{-6., 11., -6.}; + std::vector roots; + const auto num_roots = solveCubic(coeffs, roots); + + EXPECT_EQ(num_roots, 3); + EXPECT_NEAR(roots[0], 1., 1e-8); + EXPECT_NEAR(roots[1], 3., 1e-8); + EXPECT_NEAR(roots[2], 2., 1e-8); + } +} + TEST(Core_SolvePoly, regression_5599) { // x^4 - x^2 = 0, roots: 1, -1, 0, 0