You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by tn...@apache.org on 2013/08/31 21:43:02 UTC

svn commit: r1519204 - in /commons/proper/math/trunk: ./ src/changes/ src/main/java/org/apache/commons/math3/fraction/ src/test/java/org/apache/commons/math3/fraction/

Author: tn
Date: Sat Aug 31 19:43:02 2013
New Revision: 1519204

URL: http://svn.apache.org/r1519204
Log:
[MATH-996] Fix creation of Fraction/BigFraction objects in maxDenominator mode when the value is close to an actual fraction.

Modified:
    commons/proper/math/trunk/pom.xml
    commons/proper/math/trunk/src/changes/changes.xml
    commons/proper/math/trunk/src/main/java/org/apache/commons/math3/fraction/BigFraction.java
    commons/proper/math/trunk/src/main/java/org/apache/commons/math3/fraction/Fraction.java
    commons/proper/math/trunk/src/test/java/org/apache/commons/math3/fraction/BigFractionTest.java
    commons/proper/math/trunk/src/test/java/org/apache/commons/math3/fraction/FractionTest.java

Modified: commons/proper/math/trunk/pom.xml
URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/pom.xml?rev=1519204&r1=1519203&r2=1519204&view=diff
==============================================================================
--- commons/proper/math/trunk/pom.xml (original)
+++ commons/proper/math/trunk/pom.xml Sat Aug 31 19:43:02 2013
@@ -133,6 +133,9 @@
       <name>Eldar Agalarov</name>
     </contributor>
     <contributor>
+      <name>Tim Allison</name>
+    </contributor>
+    <contributor>
       <name>C. Scott Ananian</name>
     </contributor>
     <contributor>

Modified: commons/proper/math/trunk/src/changes/changes.xml
URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/src/changes/changes.xml?rev=1519204&r1=1519203&r2=1519204&view=diff
==============================================================================
--- commons/proper/math/trunk/src/changes/changes.xml (original)
+++ commons/proper/math/trunk/src/changes/changes.xml Sat Aug 31 19:43:02 2013
@@ -51,6 +51,11 @@ If the output is not quite correct, chec
   </properties>
   <body>
     <release version="x.y" date="TBD" description="TBD">
+      <action dev="tn" type=fix issue="MATH-996" due-to="Tim Allison">
+        Creating a "Fraction" or "BigFraction" object with a maxDenominator parameter
+        does not throw a "FractionConversionException" in case the value is very close
+        to fraction.
+      </action>
       <action dev="tn" type="add" issue="MATH-1028" due-to="Thorsten Schäfer">
         Added new distance metric "EarthMoversDistance".
       </action>

Modified: commons/proper/math/trunk/src/main/java/org/apache/commons/math3/fraction/BigFraction.java
URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/fraction/BigFraction.java?rev=1519204&r1=1519203&r2=1519204&view=diff
==============================================================================
--- commons/proper/math/trunk/src/main/java/org/apache/commons/math3/fraction/BigFraction.java (original)
+++ commons/proper/math/trunk/src/main/java/org/apache/commons/math3/fraction/BigFraction.java Sat Aug 31 19:43:02 2013
@@ -301,6 +301,11 @@ public class BigFraction
             p2 = (a1 * p1) + p0;
             q2 = (a1 * q1) + q0;
             if ((p2 > overflow) || (q2 > overflow)) {
+                // in maxDenominator mode, if the last fraction was very close to the actual value
+                // q2 may overflow in the next iteration; in this case return the last one.
+                if (epsilon == 0.0 && FastMath.abs(q1) < maxDenominator) {
+                    break;
+                }
                 throw new FractionConversionException(value, p2, q2);
             }
 

Modified: commons/proper/math/trunk/src/main/java/org/apache/commons/math3/fraction/Fraction.java
URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/fraction/Fraction.java?rev=1519204&r1=1519203&r2=1519204&view=diff
==============================================================================
--- commons/proper/math/trunk/src/main/java/org/apache/commons/math3/fraction/Fraction.java (original)
+++ commons/proper/math/trunk/src/main/java/org/apache/commons/math3/fraction/Fraction.java Sat Aug 31 19:43:02 2013
@@ -83,6 +83,9 @@ public class Fraction
     /** Serializable version identifier */
     private static final long serialVersionUID = 3698073679419233275L;
 
+    /** The default epsilon used for convergence. */
+    private static final double DEFAULT_EPSILON = 1e-5;
+
     /** The denominator. */
     private final int denominator;
 
@@ -96,7 +99,7 @@ public class Fraction
      *         converge.
      */
     public Fraction(double value) throws FractionConversionException {
-        this(value, 1.0e-5, 100);
+        this(value, DEFAULT_EPSILON, 100);
     }
 
     /**
@@ -182,8 +185,7 @@ public class Fraction
             throw new FractionConversionException(value, a0, 1l);
         }
 
-        // check for (almost) integer arguments, which should not go
-        // to iterations.
+        // check for (almost) integer arguments, which should not go to iterations.
         if (FastMath.abs(a0 - value) < epsilon) {
             this.numerator = (int) a0;
             this.denominator = 1;
@@ -206,7 +208,13 @@ public class Fraction
             long a1 = (long)FastMath.floor(r1);
             p2 = (a1 * p1) + p0;
             q2 = (a1 * q1) + q0;
+
             if ((FastMath.abs(p2) > overflow) || (FastMath.abs(q2) > overflow)) {
+                // in maxDenominator mode, if the last fraction was very close to the actual value
+                // q2 may overflow in the next iteration; in this case return the last one.
+                if (epsilon == 0.0 && FastMath.abs(q1) < maxDenominator) {
+                    break;
+                }
                 throw new FractionConversionException(value, p2, q2);
             }
 

Modified: commons/proper/math/trunk/src/test/java/org/apache/commons/math3/fraction/BigFractionTest.java
URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/src/test/java/org/apache/commons/math3/fraction/BigFractionTest.java?rev=1519204&r1=1519203&r2=1519204&view=diff
==============================================================================
--- commons/proper/math/trunk/src/test/java/org/apache/commons/math3/fraction/BigFractionTest.java (original)
+++ commons/proper/math/trunk/src/test/java/org/apache/commons/math3/fraction/BigFractionTest.java Sat Aug 31 19:43:02 2013
@@ -154,6 +154,9 @@ public class BigFractionTest {
         assertFraction(8, 13, new BigFraction(0.6152, 99));
         assertFraction(510, 829, new BigFraction(0.6152, 999));
         assertFraction(769, 1250, new BigFraction(0.6152, 9999));
+        
+        // MATH-996
+        assertFraction(1, 2, new BigFraction(0.5000000001, 10));
     }
 
     @Test

Modified: commons/proper/math/trunk/src/test/java/org/apache/commons/math3/fraction/FractionTest.java
URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/src/test/java/org/apache/commons/math3/fraction/FractionTest.java?rev=1519204&r1=1519203&r2=1519204&view=diff
==============================================================================
--- commons/proper/math/trunk/src/test/java/org/apache/commons/math3/fraction/FractionTest.java (original)
+++ commons/proper/math/trunk/src/test/java/org/apache/commons/math3/fraction/FractionTest.java Sat Aug 31 19:43:02 2013
@@ -129,6 +129,9 @@ public class FractionTest {
         assertFraction(8, 13,     new Fraction(0.6152,   99));
         assertFraction(510, 829,  new Fraction(0.6152,  999));
         assertFraction(769, 1250, new Fraction(0.6152, 9999));
+
+        // MATH-996
+        assertFraction(1, 2, new Fraction(0.5000000001, 10));
     }
 
     @Test
@@ -141,7 +144,9 @@ public class FractionTest {
 
     private void checkIntegerOverflow(double a) {
         try {
-            new Fraction(a, 1.0e-12, 1000);
+            @SuppressWarnings("unused")
+            Fraction f = new Fraction(a, 1.0e-12, 1000);
+            //System.out.println(f.getNumerator() + "/" + f.getDenominator());
             Assert.fail("an exception should have been thrown");
         } catch (ConvergenceException ce) {
             // expected behavior