[ https://issues.apache.org/jira/browse/MATH836?page=com.atlassian.jira.plugin.system.issuetabpanels:commenttabpanel&focusedCommentId=13426902#comment13426902
]
Thomas Neidhart edited comment on MATH836 at 8/1/12 9:26 PM:

The overflow check in Fraction does not take negative values into account, it has to be changed
to the following:
{noformat}
if (FastMath.abs(a0) > overflow) {
throw new FractionConversionException(value, a0, 1l);
}
...
if ((FastMath.abs(p2) > overflow)  (FastMath.abs(q2) > overflow)) {
throw new FractionConversionException(value, p2, q2);
}
{noformat}
In that case your examples fail correctly with a FractionConversionException and all unit
tests run through successfully.
I would be interested how you generated the test cases. Are these autogenerated?
Thanks,
Thomas
was (Author: tn):
The overflow check in Fraction does not take negative values into account, it has to be
changed to the following:
{noformat}
if ((FastMath.abs(p2) > overflow)  (FastMath.abs(q2) > overflow)) {
throw new FractionConversionException(value, p2, q2);
}
{noformat}
In that case your examples fail correctly with a FractionConversionException and all unit
tests run through successfully.
I would be interested how you generated the test cases. Are these autogenerated?
Thanks,
Thomas
> Fraction(double, int) constructor strange behaviour
> 
>
> Key: MATH836
> URL: https://issues.apache.org/jira/browse/MATH836
> Project: Commons Math
> Issue Type: Bug
> Affects Versions: 3.0
> Reporter: Baste Nesse Buanes
> Priority: Minor
> Labels: Fraction
> Attachments: FractionTestByAxiom.java, valuemaxDenominator_pairs_that_fails
>
>
> The Fraction constructor Fraction(double, int) takes a double value and a int maximal
denominator, and approximates a fraction. When the double value is a large, negative number
with many digits in the fractional part, and the maximal denominator is a big, positive integer
(in the 100'000s), two distinct bugs can manifest:
> 1: the constructor returns a positive Fraction. Calling Fraction(33655.1677817278, 371880)
returns the fraction 410517235/243036, which both has the wrong sign, and is far away from
the absolute value of the given value
> 2: the constructor does not manage to reduce the Fraction properly. Calling Fraction(43979.60679604749,
366081) returns the fraction 1651878166/256677, which should have* been reduced to 24654898/3831.
> I have, as of yet, not found a solution. The constructor looks like this:
> public Fraction(double value, int maxDenominator)
> throws FractionConversionException
> {
> this(value, 0, maxDenominator, 100);
> }
> Increasing the 100 value (max iterations) does not fix the problem for all cases. Changing
the 0value (the epsilon, maximum allowed error) to something small does not work either,
as this breaks the tests in FractionTest.
> The problem is not neccissarily that the algorithm is unable to approximate a fraction
correctly. A solution where a FractionConversionException had been thrown in each of these
examples would probably be the best solution if an improvement on the approximation algorithm
turns out to be hard to find.
> This bug has been found when trying to explore the idea of axiombased testing (http://bldl.ii.uib.no/testing.html).
Attached is a java test class FractionTestByAxiom (junit, goes into org.apache.commons.math3.fraction)
which shows these bugs through a simplified approach to this kind of testing, and a text file
describing some of the value/maxDenominator combinations which causes one of these failures.
> * It is never specified in the documentation that the Fraction class guarantees that
completely reduced rational numbers are constructed, but a comment inside the equals method
claims that "since fractions are always in lowest terms, numerators and can be compared directly
for equality", so it seems like this is the intention.

This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
