From f322f30e893341ae66e407e17ae2985d93f8735a Mon Sep 17 00:00:00 2001 From: Akash Kumar <116457960+akashchamp@users.noreply.github.com> Date: Sat, 26 Sep 2026 00:58:19 +0530 Subject: [PATCH] NUMBERS-214: Bound the default max iterations for GeneralizedContinuedFraction "GeneralizedContinuedFraction" used Integer.MAX_VALUE as the default number of iterations for the overloads of "value" (and the derived "ContinuedFraction.evaluate(double,double)") that do not accept an explicit maxIterations argument. For a fraction that does not converge this caused an ArithmeticException to be raised only after an excessive runtime (~12-15s for a simple non-converging generator on this hardware) instead of failing fast. Lower the default to 1,000,000, as suggested in the issue. A caller that requires more terms to converge can still use the existing 4-argument overloads with an explicit maxIterations; those overloads, and all current internal callers (BoostBeta, BoostGamma), already pass their own maxIterations and are unaffected by this change. Co-Authored-By: Claude Sonnet 5 --- .../numbers/fraction/ContinuedFraction.java | 4 ++ .../GeneralizedContinuedFraction.java | 27 ++++++++++++- .../GeneralizedContinuedFractionTest.java | 40 +++++++++++++++++++ 3 files changed, 69 insertions(+), 2 deletions(-) diff --git a/commons-numbers-fraction/src/main/java/org/apache/commons/numbers/fraction/ContinuedFraction.java b/commons-numbers-fraction/src/main/java/org/apache/commons/numbers/fraction/ContinuedFraction.java index ed8323c13..6feb37f34 100644 --- a/commons-numbers-fraction/src/main/java/org/apache/commons/numbers/fraction/ContinuedFraction.java +++ b/commons-numbers-fraction/src/main/java/org/apache/commons/numbers/fraction/ContinuedFraction.java @@ -71,6 +71,10 @@ public ContinuedFraction() {} /** * Evaluates the continued fraction. * + *

Uses a default limit on the number of iterations. Use + * {@link #evaluate(double,double,int)} to specify an explicit {@code maxIterations} + * for a fraction that requires more terms to converge. + * * @param x the evaluation point. * @param epsilon Maximum relative error allowed. * @return the value of the continued fraction evaluated at {@code x}. diff --git a/commons-numbers-fraction/src/main/java/org/apache/commons/numbers/fraction/GeneralizedContinuedFraction.java b/commons-numbers-fraction/src/main/java/org/apache/commons/numbers/fraction/GeneralizedContinuedFraction.java index 0b5a0882b..6e33faef1 100644 --- a/commons-numbers-fraction/src/main/java/org/apache/commons/numbers/fraction/GeneralizedContinuedFraction.java +++ b/commons-numbers-fraction/src/main/java/org/apache/commons/numbers/fraction/GeneralizedContinuedFraction.java @@ -61,8 +61,15 @@ public final class GeneralizedContinuedFraction { * eps * |b_n|, e.g., 1e-50". */ static final double SMALL = 1e-50; - /** Default maximum number of iterations. */ - static final int DEFAULT_ITERATIONS = Integer.MAX_VALUE; + /** + * Default maximum number of iterations. + * + *

This is bounded well below {@link Integer#MAX_VALUE} so that a fraction which + * does not converge fails fast with an exception rather than iterating for an + * excessive length of time. A generator that requires more terms than this to + * converge should use the overloads that accept an explicit {@code maxIterations}. + */ + static final int DEFAULT_ITERATIONS = 1_000_000; /** * Minimum relative error epsilon. Equal to 1 - Math.nextDown(1.0), or 2^-53. * @@ -149,6 +156,10 @@ private GeneralizedContinuedFraction() {} * *

Note: The first generated partial numerator a0 is discarded. * + *

Uses a default limit on the number of iterations. Use + * {@link #value(Supplier,double,int)} to specify an explicit {@code maxIterations} + * for a fraction that requires more terms to converge. + * * @param gen Generator of coefficients. * @return the value of the continued fraction. * @throws ArithmeticException if the algorithm fails to converge or if the maximal number of @@ -164,6 +175,10 @@ public static double value(Supplier gen) { * *

Note: The first generated partial numerator a0 is discarded. * + *

Uses a default limit on the number of iterations. Use + * {@link #value(Supplier,double,int)} to specify an explicit {@code maxIterations} + * for a fraction that requires more terms to converge. + * * @param gen Generator of coefficients. * @param epsilon Maximum relative error allowed. * @return the value of the continued fraction. @@ -227,6 +242,10 @@ public static double value(Supplier gen, double epsilon, int maxIte *

  • b0 is very small and the result is expected to approach zero
    • * * + *

    Uses a default limit on the number of iterations. Use + * {@link #value(double,Supplier,double,int)} to specify an explicit + * {@code maxIterations} for a fraction that requires more terms to converge. + * * @param b0 Coefficient b0. * @param gen Generator of coefficients. * @return the value of the continued fraction. @@ -250,6 +269,10 @@ public static double value(double b0, Supplier gen) { *

  • b0 is very small and the result is expected to approach zero
    • * * + *

    Uses a default limit on the number of iterations. Use + * {@link #value(double,Supplier,double,int)} to specify an explicit + * {@code maxIterations} for a fraction that requires more terms to converge. + * * @param b0 Coefficient b0. * @param gen Generator of coefficients. * @param epsilon Maximum relative error allowed. diff --git a/commons-numbers-fraction/src/test/java/org/apache/commons/numbers/fraction/GeneralizedContinuedFractionTest.java b/commons-numbers-fraction/src/test/java/org/apache/commons/numbers/fraction/GeneralizedContinuedFractionTest.java index 7eb68237b..256cad386 100644 --- a/commons-numbers-fraction/src/test/java/org/apache/commons/numbers/fraction/GeneralizedContinuedFractionTest.java +++ b/commons-numbers-fraction/src/test/java/org/apache/commons/numbers/fraction/GeneralizedContinuedFractionTest.java @@ -240,6 +240,46 @@ void testMaxIterationsThrowsB() { assertExceptionMessageContains(t, "max"); } + /** + * Test the default number of iterations is bounded well below + * {@link Integer#MAX_VALUE}. A fraction that never converges must fail fast + * using the default (no {@code maxIterations} argument) evaluation methods + * rather than run for an excessive length of time. + * + * @see NUMBERS-214 + */ + @Test + void testDefaultIterationsIsBounded() { + Assertions.assertTrue(GeneralizedContinuedFraction.DEFAULT_ITERATIONS < Integer.MAX_VALUE / 100, + () -> "Default iterations not bounded: " + GeneralizedContinuedFraction.DEFAULT_ITERATIONS); + } + + /** + * Test that evaluation of a non-converging fraction using the default number of + * iterations does not generate substantially more terms than the documented + * default limit. This bounds the runtime of a call that omits the + * {@code maxIterations} argument. + * + * @see NUMBERS-214 + */ + @Test + void testNonConvergingFractionUsesDefaultIterationLimit() { + // Oscillating generator that never converges: + // b0 = 1 seeds the evaluation (a0 is discarded); all subsequent terms + // (a=1, b=0) create a non-converging oscillation between two values. + final int[] calls = {0}; + final Supplier gen = () -> { + calls[0]++; + return Coefficient.of(1, calls[0] == 1 ? 1 : 0); + }; + + final Throwable t = Assertions.assertThrows(ArithmeticException.class, + () -> GeneralizedContinuedFraction.value(gen)); + assertExceptionMessageContains(t, "max"); + Assertions.assertTrue(calls[0] <= GeneralizedContinuedFraction.DEFAULT_ITERATIONS + 1, + () -> "Unexpected number of generator calls: " + calls[0]); + } + @Test void testNaNThrowsA() { // Create a NaN during the iteration