So, I think the code I have so far is ripe for a pull request, so I
submitted one. I changed the contracts of the epsilon and
max-denominator factory methods, because the old specifications were not
very useful, especially that of the max-denominator method – the only
guarantee you could get from it was that /some/ fraction with a
denominator not greater than the specified maximum would be returned,
without any relation to the double value that should be approximated.
The simple continued fraction is now created from an exact BigFraction
representation of the double value rather than with floating-point
arithmetic on the double value itself, to preserve maximum precision
(the old epsilon algorithm produces a fraction of 1/3 when passed the
double value obtained from the expression 1.0/3.0 and an epsilon of 5 *
2^(-58), which is incorrect because the distance between the rational
number 1/3 and its closest double representative is larger than 5 *
2^(-58); I added a corresponding unit test).
The methods setLastCoefficient(BigInteger) and removeLastCoefficient()
in the new class SimpleContinuedFraction are unused. I wrote this class
before I implemented the algorithms, and I thought these methods might
be useful in the max-denominator method, but this turned out not to be
the case. However, from a design-perspective, these two methods
complement the functionality of addCoefficient(BigInteger), so I thought
their presence is still tolerable.
I solved the problem with the maxIterations argument in the epsilon
method by simply not limiting the number of iterations if this argument
is negative. Maybe this parameter was necessary in the old algorithm
which calculated the simple continued fraction with floating-point
arithmetic, to prevent an infinite loop or something.
On 7/17/19 10:20 AM, Heinrich Bohne wrote:
It just occured to me that you might have misunderstood my sentence:
I am even more confused by your suggestion seeing as it was
you who banned BigInteger from Fraction.addSub(Fraction, boolean) in
https://issues.apache.org/jira/browse/NUMBERS-79 , which, even though
you were not aware of it at that time, did not limit the method's
functionality in any way whatsoever
The "which" was referring to your removal of BigInteger, not to the use
of BigInteger prior to your edits, so what I meant to say was, by
removing BigInteger, you did not limit the method's functionality.
On 7/17/19 10:10 AM, Heinrich Bohne wrote:
The reason it was done was because Knuth proved
(as in mathematical proof) that a long is insufficient for certain
fraction
multiplications where both numerator and denominator are large ints; 65
rather than 64 bits are necessary and a long will not suffice.
You seem to have missed my comment in ticket
https://issues.apache.org/jira/browse/NUMBERS-79 , which you created – I
don't have the book by D. Knuth, but I can only assume that the section
referenced by the code talks about unsigned integers, because by the
logic in the comment I left in the JIRA ticket, long values are
**always** sufficient in Fraction.addSub(Fraction, boolean).
But this is beside the point, I only mentioned it because I didn't
understand why you suggested to remove the BigFraction class, and
actually, I still don't, as the class BigFraction provides functionality
that Fraction cannot have, both with and without my suggested
alterations.
On 7/17/19 2:29 AM, Eric Barnhill wrote:
On Tue, Jul 16, 2019 at 2:41 PM Heinrich Bohne<heinrich.bo...@gmx.at>
wrote:
Do you think we really even need a BigFraction class at all in the
context
of these upgrades? Or should one of the Fraction factory methods just
take
BigInteger argumentsm and all fractions use the lazy dynamic
method of
calculation you are proposing?
I don't quite understand what you mean by this. The BigInteger class
provides flexibility and the ability to store and operate on
(practically) unlimited values, which Fraction does not have. The
Fraction class, on the other hand, is faster and more memory
efficient,
due to its use of primitive values, which is an advantage over
BigFraction.
That's fine.
I am even more confused by your suggestion seeing as it was
you who banned BigInteger from Fraction.addSub(Fraction, boolean) in
https://issues.apache.org/jira/browse/NUMBERS-79 , which, even though
you were not aware of it at that time, did not limit the method's
functionality in any way whatsoever (the use of int rather than long
did, however, but this is now fixed).
I don't know what you mean by "functionality" but constructing a
BigInteger
for every fraction multiplication uses up more memory and operations
than
necessary and scales poorly. BigIntegers are not fast.
However, I understand why the previous coders incorporated a
BigInteger and
I'm not sure that you do. The reason it was done was because Knuth
proved
(as in mathematical proof) that a long is insufficient for certain
fraction
multiplications where both numerator and denominator are large ints; 65
rather than 64 bits are necessary and a long will not suffice. For me,
these cases are so extreme and likely so rare that we might as well let
them fail, report to the user that these cases need to be handled with
BigFraction and leave it there. It could easily be handled in a try
catch
block and such a block would be high performance.
That was the judgment I made and it is open to interpretation, provided
such interpretation agrees with Knuth's proof. We are entitled to our
own
opinions but not our own facts.
Anyway I think your approximation schemes sound good and implement them
however you see fit.
Eric
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
