core: fix solveCubic numerical instability via coefficient normalization (fixes #27748)
Summary
This PR fixes numerical instability in cv::solveCubic when the leading coefficient a is non-zero but extremely small relative to other coefficients (Issue #27748).
It introduces a normalization step that scales all coefficients by their maximum magnitude before solving. This ensures robust detection of when the equation should degenerate to a quadratic solver, without breaking valid cubic equations that happen to have small coefficients (e.g., scaled by 1e-9).
The Problem (Issue #27748)
The previous implementation checked if (a == 0) to decide whether to use the cubic or quadratic formula.
- When
ais extremely small (e.g., 1e-17) but not exactly zero, and other coefficients are normal (e.g., 5.0), the standard cubic formula suffers from catastrophic cancellation and overflow, producing incorrect roots (e.g., 1e14).
The Fix
- Normalization: The solver now finds
max_coeff = max(|a|, |b|, |c|, |d|)and scales all coefficients by1.0 / max_coeff. - Relative Threshold: It then checks
if (abs(a) < epsilon)on the normalized coefficients.
Why this is better than previous attempts
In a previous attempt (PR #28057), a simple absolute check abs(a) < epsilon was proposed. That approach was rejected because it failed for scaled equations .
Fixes #27748
The PR introduce regression:
[ RUN ] Core_SolveCubicConstant.accuracy
/home/ci/opencv/modules/core/test/test_math.cpp:2464: Failure
Expected equality of these values:
num_roots
Which is: 0
-1
[ FAILED ] Core_SolveCubicConstant.accuracy (0 ms)
[ RUN ] Core_SolveCubicQuadratic.accuracy
/home/ci/opencv/modules/core/test/test_math.cpp:2503: Failure
Expected equality of these values:
roots[0]
Which is: 2
2.
Which is: 2
/home/ci/opencv/modules/core/test/test_math.cpp:2504: Failure
Expected equality of these values:
roots[1]
Which is: 1
1.
Which is: 1
[ FAILED ] Core_SolveCubicQuadratic.accuracy (0 ms)
[ RUN ] Core_SolveCubic.regression_27323
/home/ci/opencv/modules/core/test/test_math.cpp:2608: Failure
Expected equality of these values:
roots[0]
Which is: 1
-5e12 - 2.
Which is: -5e+12
/home/ci/opencv/modules/core/test/test_math.cpp:2617: Failure
Expected equality of these values:
roots[0]
Which is: 1
-5e12 - 2.
Which is: -5e+12
[ FAILED ] Core_SolveCubic.regression_27323 (0 ms)
Please also add test case for the issue you are solving.
The PR introduce regression:
[ RUN ] Core_SolveCubicConstant.accuracy /home/ci/opencv/modules/core/test/test_math.cpp:2464: Failure Expected equality of these values: num_roots Which is: 0 -1 [ FAILED ] Core_SolveCubicConstant.accuracy (0 ms) [ RUN ] Core_SolveCubicQuadratic.accuracy /home/ci/opencv/modules/core/test/test_math.cpp:2503: Failure Expected equality of these values: roots[0] Which is: 2 2. Which is: 2 /home/ci/opencv/modules/core/test/test_math.cpp:2504: Failure Expected equality of these values: roots[1] Which is: 1 1. Which is: 1 [ FAILED ] Core_SolveCubicQuadratic.accuracy (0 ms) [ RUN ] Core_SolveCubic.regression_27323 /home/ci/opencv/modules/core/test/test_math.cpp:2608: Failure Expected equality of these values: roots[0] Which is: 1 -5e12 - 2. Which is: -5e+12 /home/ci/opencv/modules/core/test/test_math.cpp:2617: Failure Expected equality of these values: roots[0] Which is: 1 -5e12 - 2. Which is: -5e+12 [ FAILED ] Core_SolveCubic.regression_27323 (0 ms)Please also add test case for the issue you are solving.
Thank you for the feedback.
-
Fixed Regressions:
Core_SolveCubicQuadratic(Precision):I modified the logic to use the original (raw) coefficients when falling back to the quadratic solver. This prevents the floating-point rounding errors that were causing the exact-match tests to fail.Core_SolveCubicConstant(Infinite Roots): Added an explicit check for all-zero coefficients to correctly return-1(infinite roots) as expected by the tests.Core_SolveCubic.regression_27323(Small Valid Cubic): I updated the threshold logic to usestd::numeric_limits<double>::epsilon()on normalized coefficients. This ensures that valid cubic equations with small but significant terms are not incorrectly degraded to quadratic. -
Test Case Added: I have added
TEST(Core_SolveCubic, regression_27748)totest_math.cpp, which specifically targets the numerical instability reported in the issue. -
CI Fix: Added missing
<algorithm>and<cmath>headers to fix the build failures on Linux/Windows CI agents.
CI Warnings:
**modules/core/src/mathfuncs.cpp:49: trailing whitespace.
+#include <algorithm>
modules/core/src/mathfuncs.cpp:50: trailing whitespace.
+#include <cmath>
modules/core/src/mathfuncs.cpp:1606: trailing whitespace.
+ // CRITICAL: Use RAW coefficients here to avoid rounding errors in trivial cases
modules/core/src/mathfuncs.cpp:1650: trailing whitespace.
+ double a0 = a0_norm;
modules/core/src/mathfuncs.cpp:1663: trailing whitespace.
+
modules/core/test/test_math.cpp:2623: trailing whitespace.
+
modules/core/test/test_math.cpp:2624: trailing whitespace.
+ // a is extremely small relative to others (approx 1.8e-19 ratio),
modules/core/test/test_math.cpp:2633: trailing whitespace.
+
modules/core/test/test_math.cpp:2638: trailing whitespace.
+
modules/core/test/test_math.cpp:2642: trailing whitespace.
+
modules/core/test/test_math.cpp:2646: trailing whitespace.
+ EXPECT_NEAR(r, 0.5, 1.0);
CI Warnings:
**modules/core/src/mathfuncs.cpp:49: trailing whitespace. +#include <algorithm> modules/core/src/mathfuncs.cpp:50: trailing whitespace. +#include <cmath> modules/core/src/mathfuncs.cpp:1606: trailing whitespace. + // CRITICAL: Use RAW coefficients here to avoid rounding errors in trivial cases modules/core/src/mathfuncs.cpp:1650: trailing whitespace. + double a0 = a0_norm; modules/core/src/mathfuncs.cpp:1663: trailing whitespace. + modules/core/test/test_math.cpp:2623: trailing whitespace. + modules/core/test/test_math.cpp:2624: trailing whitespace. + // a is extremely small relative to others (approx 1.8e-19 ratio), modules/core/test/test_math.cpp:2633: trailing whitespace. + modules/core/test/test_math.cpp:2638: trailing whitespace. + modules/core/test/test_math.cpp:2642: trailing whitespace. + modules/core/test/test_math.cpp:2646: trailing whitespace. + EXPECT_NEAR(r, 0.5, 1.0);
Apologies for the build failures. I will pushed a fix that:
- Removes trailing whitespace to pass the style check.
- Adds missing headers (
<algorithm>,<cmath>) for cross-platform compilation.
Sorry I used ai for removing whitespace which occur in docs previous once again sorry for forward I will make sure of this.
For inconvenience you can close my PR
On Thu, 4 Dec 2025, 23:38 s-trinh, @.***> wrote:
@.**** commented on this pull request.
In modules/core/src/mathfuncs.cpp https://github.com/opencv/opencv/pull/28117#discussion_r2590080223:
/*
Here we expand expression `Qcubed - R * R` for `d` variableto reduce common terms `a1^6 / 729` and `-a1^4 * a2 / 81`and thus decrease rounding error (in case of quite big coefficients).And then we additionally group terms to further reduce rounding error.*/Please, do not remove old comments.
BTW, too much AI code / comments. No idea if the approach is valid or not, or if the comments match with the code.
— Reply to this email directly, view it on GitHub https://github.com/opencv/opencv/pull/28117#pullrequestreview-3541389583, or unsubscribe https://github.com/notifications/unsubscribe-auth/BMZMNSHLGSG422EJ6SKHPV34AB2B5AVCNFSM6AAAAACNZXSE46VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTKNBRGM4DSNJYGM . You are receiving this because you authored the thread.Message ID: @.***>
Sure I will remember all these things
On Fri, 5 Dec 2025, 10:54 Alexander Smorkalov, @.***> wrote:
@.**** commented on this pull request.
In modules/core/test/test_math.cpp https://github.com/opencv/opencv/pull/28117#discussion_r2591438794:
- Mat coeffs = (Mat_
(1, 4) << a, b, c, d);
- Mat roots;
- // We expect the solver to detect the negligible 'a' and solve as quadratic:
- // 84.4504x^2 - 96.795x + 13.6826 = 0
- // Discriminant ~ 4745.8
- // Roots approx: 0.156, 0.990
- int n = solveCubic(coeffs, roots);
- EXPECT_GE(n, 2); // Should find at least 2 real roots (as quadratic)
- for(int i = 0; i < n; i++)
- {
double r = roots.at<double>(i);EXPECT_NEAR(r, 0.5, 1.0);It means that roots are validated in +-1 range. It's too much. See documentation: https://google.github.io/googletest/reference/assertions.html#EXPECT_NEAR
In modules/core/src/mathfuncs.cpp https://github.com/opencv/opencv/pull/28117#discussion_r2591442231:
double theta = acos(R / sqrt(Qcubed));
double sqrtQ = sqrt(Q);Please do not add std:: to the code you do not touch. It produces a lot of noise.
— Reply to this email directly, view it on GitHub https://github.com/opencv/opencv/pull/28117#pullrequestreview-3543151102, or unsubscribe https://github.com/notifications/unsubscribe-auth/BMZMNSFHA2JXR2MTRGG2NA34AEJKFAVCNFSM6AAAAACNZXSE46VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTKNBTGE2TCMJQGI . You are receiving this because you authored the thread.Message ID: @.***>
Ok, I will do that.
On Fri, 5 Dec 2025, 13:06 Vadim Pisarevsky, @.***> wrote:
@.**** requested changes on this pull request.
please, reduce the patch size as much as possible, i.e. remove unnecessary changes, makes the comments less verbose, keep the old code and comments where it's possible.
— Reply to this email directly, view it on GitHub https://github.com/opencv/opencv/pull/28117#pullrequestreview-3543454253, or unsubscribe https://github.com/notifications/unsubscribe-auth/BMZMNSAZGFR5UXWSIF6BB6L4AEYYVAVCNFSM6AAAAACNZXSE46VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTKNBTGQ2TIMRVGM . You are receiving this because you authored the thread.Message ID: @.***>