[jira] [Commented] (NUMBERS-99) Fraction.add(int) and Fraction.subtract(int) ignore risk of integer overflow
[
https://issues.apache.org/jira/browse/NUMBERS-99?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16945475#comment-16945475
]
Heinrich Bohne commented on NUMBERS-99:
---
Hello,
sorry for having been absent for so long, I was a bit more preoccupied than I
anticipated. In fact, I still am, in a way, as I don't have as much time as I
used to, but I also don't want to leave all the open issues and pull requests
that I created just lying around.
About your question: If the denominator instance variable is always positive,
then either the contract of the factory method {{of(int, int)}} needs to be
changed to declare when the exception will be thrown, or another field must be
introduced that would cause the representation of a fraction to resemble a
signed-magnitude-representation. In the latter case, it would probably be
better to require the denominator to be always negative instead of positive, to
increase the number of possible denominator or numerator values by one.
But I don't think a signed-magnitude-representation makes things easier than
allowing both the numerator and the denominator to be negative OR positive, and
it would probably require more refactoring. So I think it will be either
changing the public contract, or allowing the denominator variable to be
negative.
What would be useful in any case would be a signum function method (I think you
suggested something like this somewhere), because even currently, there is no
publicly guaranteed way of determining the sign of a fraction other than
obtaining both the numerator and the denominator (because the denominator
reflecting the sign of the fraction is an implementation detail).
> Fraction.add(int) and Fraction.subtract(int) ignore risk of integer overflow
>
>
> Key: NUMBERS-99
> URL: https://issues.apache.org/jira/browse/NUMBERS-99
> Project: Commons Numbers
> Issue Type: Bug
> Components: fraction
>Reporter: Heinrich Bohne
>Priority: Minor
> Time Spent: 20m
> Remaining Estimate: 0h
>
> The methods {{add(int)}} and {{subtract(int)}} in the class
> {{org.apache.commons.numbers.fraction.Fraction}} do not take into account the
> risk of an integer overflow. For example, (2^31^ - 1)/2 + 1 = (2^31^ +
> 1)/2, so the numerator overflows an {{int}}, but when calculated with
> {{Fraction.add(int)}}, the method still returns normally.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (NUMBERS-99) Fraction.add(int) and Fraction.subtract(int) ignore risk of integer overflow
[
https://issues.apache.org/jira/browse/NUMBERS-99?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16903381#comment-16903381
]
Heinrich Bohne commented on NUMBERS-99:
---
You're right, this issue is still open. However, the problem is that I found a
gaping hole in the public contract of the {{Fraction}} class: Since the
positivity of the denominator is an implementation detail, the exact
circumstances under which the factory method {{of(int, int)}} will throw an
{{ArithmeticException}} remain a mystery to the user, which means that this
factory method is essentially unpredictable when only going by the
specification and not the implementation.
I thought that this might be solvable by simply _not_ requiring the stored
denominator to be positive, but this would probably break a lot of
functionality whose implementation would have to be adjusted, so this should be
discussed on the dev mailing list. But since the resolution of this bug would
also depend on what course of action will be taken with regard to the issue I
just mentioned, I thought it might be better to leave this issue open, for now.
> Fraction.add(int) and Fraction.subtract(int) ignore risk of integer overflow
>
>
> Key: NUMBERS-99
> URL: https://issues.apache.org/jira/browse/NUMBERS-99
> Project: Commons Numbers
> Issue Type: Bug
> Components: fraction
>Reporter: Heinrich Bohne
>Priority: Minor
> Time Spent: 20m
> Remaining Estimate: 0h
>
> The methods {{add(int)}} and {{subtract(int)}} in the class
> {{org.apache.commons.numbers.fraction.Fraction}} do not take into account the
> risk of an integer overflow. For example, (2^31^ - 1)/2 + 1 = (2^31^ +
> 1)/2, so the numerator overflows an {{int}}, but when calculated with
> {{Fraction.add(int)}}, the method still returns normally.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Resolved] (NUMBERS-120) Major loss of precision in BigFraction.doubleValue() and BigFraction.floatValue()
[
https://issues.apache.org/jira/browse/NUMBERS-120?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Heinrich Bohne resolved NUMBERS-120.
Resolution: Fixed
Fix Version/s: 1.0
> Major loss of precision in BigFraction.doubleValue() and
> BigFraction.floatValue()
> -
>
> Key: NUMBERS-120
> URL: https://issues.apache.org/jira/browse/NUMBERS-120
> Project: Commons Numbers
> Issue Type: Bug
> Components: fraction
>Affects Versions: 1.0
>Reporter: Heinrich Bohne
>Priority: Minor
> Fix For: 1.0
>
> Time Spent: 1h 10m
> Remaining Estimate: 0h
>
> The method {{BigFraction.doubleValue()}} calculates the double value of
> fractions with numerators or denominators that, when converted to a
> {{double}}, round up to {{Double.POSITIVE_INFINITY}}, by right-shifting both
> the numerator and denominator synchronously until both numbers fit into 1023
> bits. Apart from the fact that the maximum number of bits an integer
> representable as a finite {{double}} can have is 1024 (an unbiased exponent
> of 1023, which is the largest possible unbiased exponent of a {{double}}
> number, means 1. ⋅ 2^1023^, which amounts to 1024 bits), this way of
> converting the fraction to a {{double}} is incredibly wasteful with precision
> if the numerator and denominator have a different bit length, because the
> smaller of the two numbers will be truncated beyond what is necessary to
> represent it as a finite {{double}}. Here is an extreme example:
> The smallest integer that rounds up to {{Double.POSITIVE_INFINITY}} when
> converted to a {{double}} is 2^1024^ - 2^970^. This is because
> {{Double.MAX_VALUE}} as an integer is a 1024-bit number with the most
> significant 53 bits set to 1 and all other 971 bits set to 0. If the 970
> least significant bits are changed in any way, the number will still round
> down to {{Double.MAX_VALUE}} as long as the 971st bit remains 0, but as soon
> as the 971st bit is set to 1, the number will round up to
> {{Double.POSITIVE_INFINITY}}.
> The smallest possible denominator greater than 1 where a single right-shift
> will cause a loss of precision is 3. 2^1024^ - 2^970^ is divisible by 3, so
> in order to create an irreducible fraction, let's add 1 to it:
> (2^1024^ - 2^970^ + 1) / 3 ≈ 5.992310449541053 ⋅ 10^307^ (which can be
> verified with {{BigDecimal}}, or, more easily, with [this online
> tool|https://www.wolframalpha.com/input/?i=(2%5E1024+-+2%5E970+%2B+1)+%2F+3].
> However, the current implementation of BigFraction.doubleValue() returns
> 8.98846567431158 ⋅ 10^307^, which differs from the correct result by a
> relative error of 50%! The same problem applies to the method
> {{BigFraction.floatValue()}}.
> This can be prevented by truncating the numerator and denominator separately,
> so that for each of the two numbers, the maximum possible precision is
> retained.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Resolved] (NUMBERS-132) ArithmeticUtils.gcd(int, int) can be simplified by performing the gcd algorithm on negative numbers
[
https://issues.apache.org/jira/browse/NUMBERS-132?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Heinrich Bohne resolved NUMBERS-132.
Resolution: Fixed
Fix Version/s: 1.0
> ArithmeticUtils.gcd(int, int) can be simplified by performing the gcd
> algorithm on negative numbers
> ---
>
> Key: NUMBERS-132
> URL: https://issues.apache.org/jira/browse/NUMBERS-132
> Project: Commons Numbers
> Issue Type: Improvement
> Components: core
>Affects Versions: 1.0
>Reporter: Heinrich Bohne
>Priority: Minor
> Fix For: 1.0
>
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> The method {{ArithmeticUtils.gcd(int, int)}} currently handles the special
> case of the non-negatable {{Integer.MIN_VALUE}} by converting the arguments
> to {{long}}s if one of them is {{Integer.MIN_VALUE}} and performing two
> iterations of the regular euclidean algorithm before handing the resulting
> values over to a helper method that performs the binary gcd algorithm.
> However, the tactic used by {{gcd(long, long)}} is much more elegant: It just
> converts positive arguments to their negative counterparts, thereby avoiding
> the risk of overflow completely without having to make exceptions for special
> cases and resorting to other data types.
> The method {{gcd(int, int)}} would likely be much more compact if it also
> were to apply this technique.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (NUMBERS-123) "BigFraction(double)" is unnecessary
[
https://issues.apache.org/jira/browse/NUMBERS-123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16903364#comment-16903364
]
Heinrich Bohne commented on NUMBERS-123:
Sorry for the confusion, the "no" was referring to my perceived need to discuss
this matter further, so if this is the determining factor, then "yes", the
issue can be marked as resolved :)
> "BigFraction(double)" is unnecessary
>
>
> Key: NUMBERS-123
> URL: https://issues.apache.org/jira/browse/NUMBERS-123
> Project: Commons Numbers
> Issue Type: Improvement
> Components: fraction
>Reporter: Gilles
>Assignee: Gilles
>Priority: Trivial
> Fix For: 1.0
>
> Attachments: NUMBERS-123__Javadoc.patch
>
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> Constructor {{BigFraction(double value)}} is only called from the
> {{from(double value)}} method.
> Actually, this constructor is misleading as it is indeed primarily a
> conversion from which appropriate {{numerator}} and {{denominator}} fields
> are computed; those could be set by
> the "direct" constructor {{BigFraction(BigInteger num, BigInteger den)}}.
> Moreover, the private field {{ZERO}} goes through this conversion code
> whereas it could constructed "directly", e.g. using {{of(0)}}. Similarly for
> field {{ONE}}.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (NUMBERS-133) Speed up Primes.nextPrime(int)
[
https://issues.apache.org/jira/browse/NUMBERS-133?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16903358#comment-16903358
]
Heinrich Bohne commented on NUMBERS-133:
I suppose you mean the modular congruence operators ≡ and ≢? Yes, sure. So
should MathJax also be used for documentation of {{private}} or package-private
elements? Because if that's the case, I'll need to update the documentation in
[PR #66|https://github.com/apache/commons-numbers/pull/66], where I used html
tags instead of MathJax. The reason I didn't use MathJax in {{private}} or
package-private documentation is that, as far as I understand, IDEs generally
don't render the MathJax, so the documentation is unlikely to be read by anyone
in a format where the MathJax will be correctly displayed. But I can convert
the documentation to MathJax, no problem.
> Speed up Primes.nextPrime(int)
> --
>
> Key: NUMBERS-133
> URL: https://issues.apache.org/jira/browse/NUMBERS-133
> Project: Commons Numbers
> Issue Type: Improvement
> Components: primes
>Affects Versions: 1.0
>Reporter: Heinrich Bohne
>Priority: Minor
> Time Spent: 50m
> Remaining Estimate: 0h
>
> The method {{Primes.nextPrime(int)}} can use the same algorithm to skip
> multiples of certain primes as {{SmallPrimes.boundedTrialDivision(int, int,
> List)}} uses, instead of hard-coding the alternating increment of
> the trial candidate into a loop.
> Also, if the argument of the method is smaller than or equal to the 512th
> prime number, the method can just infer the next higher prime number directly
> from the array {{SmallPrimes.PRIMES}} without performing any calculations.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (NUMBERS-123) "BigFraction(double)" is unnecessary
[
https://issues.apache.org/jira/browse/NUMBERS-123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16903341#comment-16903341
]
Heinrich Bohne commented on NUMBERS-123:
If you mean whether I want to discuss the matter of constructor vs. factory
method further, then no, I realize there are certain advantages that factory
methods have over constructors. Should we, at some point in the future, find
that the most pressing issue in this project at that time is that the
validation and the reduction of the numerator and denominator is performed
(whether implicitly or explicitly) both in the factory methods _and_ in the
constructor, we can always open a separate JIRA ticket for that :D
> "BigFraction(double)" is unnecessary
>
>
> Key: NUMBERS-123
> URL: https://issues.apache.org/jira/browse/NUMBERS-123
> Project: Commons Numbers
> Issue Type: Improvement
> Components: fraction
>Reporter: Gilles
>Assignee: Gilles
>Priority: Trivial
> Fix For: 1.0
>
> Attachments: NUMBERS-123__Javadoc.patch
>
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> Constructor {{BigFraction(double value)}} is only called from the
> {{from(double value)}} method.
> Actually, this constructor is misleading as it is indeed primarily a
> conversion from which appropriate {{numerator}} and {{denominator}} fields
> are computed; those could be set by
> the "direct" constructor {{BigFraction(BigInteger num, BigInteger den)}}.
> Moreover, the private field {{ZERO}} goes through this conversion code
> whereas it could constructed "directly", e.g. using {{of(0)}}. Similarly for
> field {{ONE}}.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Created] (NUMBERS-133) Speed up Primes.nextPrime(int)
Heinrich Bohne created NUMBERS-133:
--
Summary: Speed up Primes.nextPrime(int)
Key: NUMBERS-133
URL: https://issues.apache.org/jira/browse/NUMBERS-133
Project: Commons Numbers
Issue Type: Improvement
Components: primes
Affects Versions: 1.0
Reporter: Heinrich Bohne
The method {{Primes.nextPrime(int)}} can use the same algorithm to skip
multiples of certain primes as {{SmallPrimes.boundedTrialDivision(int, int,
List)}} uses, instead of hard-coding the alternating increment of the
trial candidate into a loop.
Also, if the argument of the method is smaller than or equal to the 512th prime
number, the method can just infer the next higher prime number directly from
the array {{SmallPrimes.PRIMES}} without performing any calculations.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Created] (NUMBERS-132) ArithmeticUtils.gcd(int, int) can be simplified by performing the gcd algorithm on negative numbers
Heinrich Bohne created NUMBERS-132:
--
Summary: ArithmeticUtils.gcd(int, int) can be simplified by
performing the gcd algorithm on negative numbers
Key: NUMBERS-132
URL: https://issues.apache.org/jira/browse/NUMBERS-132
Project: Commons Numbers
Issue Type: Improvement
Components: core
Affects Versions: 1.0
Reporter: Heinrich Bohne
The method {{ArithmeticUtils.gcd(int, int)}} currently handles the special case
of the non-negatable {{Integer.MIN_VALUE}} by converting the arguments to
{{long}}s if one of them is {{Integer.MIN_VALUE}} and performing two
iterations of the regular euclidean algorithm before handing the resulting
values over to a helper method that performs the binary gcd algorithm.
However, the tactic used by {{gcd(long, long)}} is much more elegant: It just
converts positive arguments to their negative counterparts, thereby avoiding
the risk of overflow completely without having to make exceptions for special
cases and resorting to other data types.
The method {{gcd(int, int)}} would likely be much more compact if it also were
to apply this technique.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Created] (NUMBERS-131) Re-designing BigFraction.from(double, double, int, int)
Heinrich Bohne created NUMBERS-131:
--
Summary: Re-designing BigFraction.from(double, double, int, int)
Key: NUMBERS-131
URL: https://issues.apache.org/jira/browse/NUMBERS-131
Project: Commons Numbers
Issue Type: Improvement
Components: fraction
Affects Versions: 1.0
Reporter: Heinrich Bohne
The method {{BigFraction.from(double, double, int, int)}} can be improved in
several ways:
* It only allows a maximum denominator in the {{int}} range, which defies the
purpose of having a {{BigFraction}} class in addition to the class
{{Fraction}}. Since {{BigFraction}} is {{BigInteger}} based, it would only be
natural to allow the maximum denominator to be passed as a {{BigInteger}}.
* It only calculates the convergents of the simple continued fraction, but not
its semi-convergents, so it doesn't necessarily produce the closest possible
approximation within the given bounds.
* The design is awkward. Making the method's behavior dependent on the values
of its arguments is confusing, even the documentation acknowledges this.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Resolved] (NUMBERS-130) Test classes and methods can be package-private
[
https://issues.apache.org/jira/browse/NUMBERS-130?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Heinrich Bohne resolved NUMBERS-130.
Resolution: Not A Problem
Fix Version/s: 1.0
> Test classes and methods can be package-private
> ---
>
> Key: NUMBERS-130
> URL: https://issues.apache.org/jira/browse/NUMBERS-130
> Project: Commons Numbers
> Issue Type: Improvement
>Affects Versions: 1.0
>Reporter: Heinrich Bohne
>Priority: Minor
> Fix For: 1.0
>
> Time Spent: 20m
> Remaining Estimate: 0h
>
> With JUnit 5, test classes and methods don't need to be {{public}}, unlike
> with JUnit 4. It suffices if they are package-private.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (NUMBERS-130) Test classes and methods can be package-private
[
https://issues.apache.org/jira/browse/NUMBERS-130?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16881334#comment-16881334
]
Heinrich Bohne commented on NUMBERS-130:
Yes, it does not really matter; to be honest, I just wanted to make the IDE
warnings go away (of course, this does not mean that I would not find a
package-private access preferable over a {{public}} access without the IDE
warnings, since I think it's generally a good idea to reduce scope to the
smallest extent necessary, so it's not like I intended to sacrifice code design
just to shut the IDE up, but I probably wouldn't have cared enough to do
anything about it). But it seems that, in the cases you described, the
practical benefits of leaving the test methods and classes {{public}} outweigh
whatever would be gained from making them package-private (which would be
probably more of a cosmetic nature than anything of real practical use).
> Test classes and methods can be package-private
> ---
>
> Key: NUMBERS-130
> URL: https://issues.apache.org/jira/browse/NUMBERS-130
> Project: Commons Numbers
> Issue Type: Improvement
>Affects Versions: 1.0
>Reporter: Heinrich Bohne
>Priority: Minor
> Time Spent: 20m
> Remaining Estimate: 0h
>
> With JUnit 5, test classes and methods don't need to be {{public}}, unlike
> with JUnit 4. It suffices if they are package-private.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Created] (NUMBERS-130) Test classes and methods can be package-private
Heinrich Bohne created NUMBERS-130:
--
Summary: Test classes and methods can be package-private
Key: NUMBERS-130
URL: https://issues.apache.org/jira/browse/NUMBERS-130
Project: Commons Numbers
Issue Type: Improvement
Affects Versions: 1.0
Reporter: Heinrich Bohne
With JUnit 5, test classes and methods don't need to be {{public}}, unlike with
JUnit 4. It suffices if they are package-private.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (NUMBERS-129) Waste of functionality in Fraction.addSub(Fraction, boolean)
[
https://issues.apache.org/jira/browse/NUMBERS-129?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16879573#comment-16879573
]
Heinrich Bohne commented on NUMBERS-129:
Thanks. I see that you changed the documentation whenever it said that a
"{{Fraction}} instance" is returned to instead specify that a "new instance" is
returned. But is this not an implementation detail unless it concerns a
constructor? In fact, it is not even always true, for example,
{{addSub(Fraction, boolean)}} returns the argument when {{this}} is zero and
{{isAdd}} is {{true}}, and it returns {{this}} when the argument is zero.
Likewise, {{multiply(Fraction)}} returns the constant {{ZERO}} if one of the
two factors is zero.
> Waste of functionality in Fraction.addSub(Fraction, boolean)
>
>
> Key: NUMBERS-129
> URL: https://issues.apache.org/jira/browse/NUMBERS-129
> Project: Commons Numbers
> Issue Type: Bug
> Components: fraction
>Affects Versions: 1.0
>Reporter: Heinrich Bohne
>Priority: Minor
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> [NUMBERS-79|https://issues.apache.org/jira/browse/NUMBERS-79], which has been
> marked as resolved by now, suggested to replace the usage of {{BigInteger}}
> in the method {{Fraction.addSub(Fraction, boolean)}} with primitive {{long}}
> values for the sake of performance. However, the ticket's
> "[resolution|https://github.com/apache/commons-numbers/commit/7c2486362b64201fd8dc19b2df12aefba2a4165a];
> does not use {{long}}, but {{int}} variables (as I hinted at in a comment in
> the ticket about a month ago, when the code was still in the branch
> fraction-dev and not merged into master yet), although the method's
> documentation was changed to state that the method uses the {{long}} datatype.
> So for one thing, the method's documentation makes claims about the method's
> implementation that are simply not true, and for another, the usage of
> {{int}}s can cause the method to fail, while using {{long}}s would _always_
> produce the correct result because an overflow would never occur (which I
> also explained in the aforementioned comment in the ticket).
> As for performance: I doubt that three {{int}}\-to\-{{long}} conversions
> and subsequent multiplications and one {{Math.toIntExact(long)}} invocation
> are significantly less performant than three {{Math.multiplyExact(int, int)}}
> and one {{Math.addExact(int, int)}} invocation (if at all), so using
> {{int}}s rather than {{long}}s sacrifices functionality for practically
> nothing.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Resolved] (NUMBERS-129) Waste of functionality in Fraction.addSub(Fraction, boolean)
[
https://issues.apache.org/jira/browse/NUMBERS-129?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Heinrich Bohne resolved NUMBERS-129.
Resolution: Fixed
Fix Version/s: 1.0
> Waste of functionality in Fraction.addSub(Fraction, boolean)
>
>
> Key: NUMBERS-129
> URL: https://issues.apache.org/jira/browse/NUMBERS-129
> Project: Commons Numbers
> Issue Type: Bug
> Components: fraction
>Affects Versions: 1.0
>Reporter: Heinrich Bohne
>Priority: Minor
> Fix For: 1.0
>
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> [NUMBERS-79|https://issues.apache.org/jira/browse/NUMBERS-79], which has been
> marked as resolved by now, suggested to replace the usage of {{BigInteger}}
> in the method {{Fraction.addSub(Fraction, boolean)}} with primitive {{long}}
> values for the sake of performance. However, the ticket's
> "[resolution|https://github.com/apache/commons-numbers/commit/7c2486362b64201fd8dc19b2df12aefba2a4165a];
> does not use {{long}}, but {{int}} variables (as I hinted at in a comment in
> the ticket about a month ago, when the code was still in the branch
> fraction-dev and not merged into master yet), although the method's
> documentation was changed to state that the method uses the {{long}} datatype.
> So for one thing, the method's documentation makes claims about the method's
> implementation that are simply not true, and for another, the usage of
> {{int}}s can cause the method to fail, while using {{long}}s would _always_
> produce the correct result because an overflow would never occur (which I
> also explained in the aforementioned comment in the ticket).
> As for performance: I doubt that three {{int}}\-to\-{{long}} conversions
> and subsequent multiplications and one {{Math.toIntExact(long)}} invocation
> are significantly less performant than three {{Math.multiplyExact(int, int)}}
> and one {{Math.addExact(int, int)}} invocation (if at all), so using
> {{int}}s rather than {{long}}s sacrifices functionality for practically
> nothing.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Created] (NUMBERS-129) Waste of functionality in Fraction.addSub(Fraction, boolean)
Heinrich Bohne created NUMBERS-129:
--
Summary: Waste of functionality in Fraction.addSub(Fraction,
boolean)
Key: NUMBERS-129
URL: https://issues.apache.org/jira/browse/NUMBERS-129
Project: Commons Numbers
Issue Type: Bug
Components: fraction
Affects Versions: 1.0
Reporter: Heinrich Bohne
[NUMBERS-79|https://issues.apache.org/jira/browse/NUMBERS-79], which has been
marked as resolved by now, suggested to replace the usage of {{BigInteger}} in
the method {{Fraction.addSub(Fraction, boolean)}} with primitive {{long}}
values for the sake of performance. However, the ticket's
"[resolution|https://github.com/apache/commons-numbers/commit/7c2486362b64201fd8dc19b2df12aefba2a4165a];
does not use {{long}}, but {{int}} variables (as I hinted at in a comment in
the ticket about a month ago, when the code was still in the branch
fraction-dev and not merged into master yet), although the method's
documentation was changed to state that the method uses the {{long}} datatype.
So for one thing, the method's documentation makes claims about the method's
implementation that are simply not true, and for another, the usage of
{{int}}s can cause the method to fail, while using {{long}}s would _always_
produce the correct result because an overflow would never occur (which I also
explained in the aforementioned comment in the ticket).
As for performance: I doubt that three {{int}}\-to\-{{long}} conversions and
subsequent multiplications and one {{Math.toIntExact(long)}} invocation are
significantly less performant than three {{Math.multiplyExact(int, int)}} and
one {{Math.addExact(int, int)}} invocation (if at all), so using {{int}}s
rather than {{long}}s sacrifices functionality for practically nothing.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Created] (NUMBERS-128) Precision of Fraction.floatValue() can be improved
Heinrich Bohne created NUMBERS-128:
--
Summary: Precision of Fraction.floatValue() can be improved
Key: NUMBERS-128
URL: https://issues.apache.org/jira/browse/NUMBERS-128
Project: Commons Numbers
Issue Type: Improvement
Components: fraction
Affects Versions: 1.0
Reporter: Heinrich Bohne
The current implementation of {{Fraction.floatValue()}} first calls
{{doubleValue()}}, which converts the fraction to a {{double}} with maximum
possible precision, and then rounds the returned {{double}} value to a
{{float}}. For the fraction 1 + 2^6^ / (2^30^ - 1) = (2^30^ - 1 + 2^6^) /
(2^30^ - 1), this yields a result of exactly 1. However, the [actual value of
the
fraction|https://www.wolframalpha.com/input/?i=(2%5E30+-+1+%2B+2%5E6)%2F(2%5E30+-+1)+to+base+2]
is closer to 1 + 2^-23^ (which is representable exactly as a {{float}}) than
it is to 1, as WolframAlpha
[confirms|https://www.wolframalpha.com/input/?i=abs((2%5E30+-+1+%2B+2%5E6)%2F(2%5E30+-+1)+-+(1+%2B+2%5E-23))+%3C+abs((2%5E30+-+1+%2B+2%5E6)%2F(2%5E30+-+1)+-+1)].
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (NUMBERS-123) "BigFraction(double)" is unnecessary
[
https://issues.apache.org/jira/browse/NUMBERS-123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16877317#comment-16877317
]
Heinrich Bohne commented on NUMBERS-123:
Actually, it's not really that big of a deal, so I'd suggest we just leave it
at that. Personally, I would still prefer the old design, but I think the
length of the discussion about this is starting to outweigh the actual
significance of the problem, especially seeing as there are other, undeniably
more significant issues to be resolved in this project.
I do realize, however, that having the responsibility of ensuring the validity
of the fields located in one place is nice, so this is a benefit of having a
single constructor.
> "BigFraction(double)" is unnecessary
>
>
> Key: NUMBERS-123
> URL: https://issues.apache.org/jira/browse/NUMBERS-123
> Project: Commons Numbers
> Issue Type: Improvement
> Components: fraction
>Reporter: Gilles
>Assignee: Gilles
>Priority: Trivial
> Fix For: 1.0
>
> Attachments: NUMBERS-123__Javadoc.patch
>
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> Constructor {{BigFraction(double value)}} is only called from the
> {{from(double value)}} method.
> Actually, this constructor is misleading as it is indeed primarily a
> conversion from which appropriate {{numerator}} and {{denominator}} fields
> are computed; those could be set by
> the "direct" constructor {{BigFraction(BigInteger num, BigInteger den)}}.
> Moreover, the private field {{ZERO}} goes through this conversion code
> whereas it could constructed "directly", e.g. using {{of(0)}}. Similarly for
> field {{ONE}}.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Comment Edited] (NUMBERS-123) "BigFraction(double)" is unnecessary
[
https://issues.apache.org/jira/browse/NUMBERS-123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16876910#comment-16876910
]
Heinrich Bohne edited comment on NUMBERS-123 at 7/2/19 9:32 PM:
bq. The sole purpose of the flag is efficiency
Actually, I wasn't concerned about efficiency at all when I raised this issue.
I found the design itself a bit awkward, because I think code should not do
more than it needs to do; efficiency is only a by-product of this. And because
I think that it is indeed the responsibility of the constructor (whether it is
private or public) to validate the values to which it initializes the fields, I
don't think the flag-approach is a satisfactory solution (of course, one could
include a warning in the documentation of the constructor that it must only be
called with valid arguments, but then, I really don't see how this would be
better than having three separate constructors).
bq. this discussion also is a hint that the initial setup was not clear
Indeed, it was not obvious that the three constructors all reduced the fraction
to lowest terms. A comment would probably have been in order. Although I don't
see what this has to do with the awkward initialization of the ZERO and ONE
constants. As far as I can tell, this is simply a matter of which
method/constructor to call – converting the constructors to factory methods
should not make a difference in this regard.
In the end, I think it boils down to the question of whether one is willing to
entrust the code that converts a double value to a fraction with the
responsibility of ensuring that the calculated numerator and denominator
fulfill the conditions required by the fields' contracts (i.e. lowest terms,
sign in numerator etc.), which is not unreasonable, since the two double
constructors/methods accomplish this without code duplication. If yes, then the
code would be better of in constructors, like in the original design. If not,
then the current design seems the most adequate.
was (Author: schamschi):
bq. The sole purpose of the flag is efficiency
Actually, I wasn't concerned about efficiency at all when I raised this issue.
I found the design itself a bit awkward, because I think code should not do
more than it needs to do; efficiency is only a by-product of this. And because
I think that it is indeed the responsibility of the constructor (whether it is
private or public) to validate the values to which it initializes the fields, I
don't think the flag-approach is a satisfactory solution (of course, one could
include a warning in the documentation of the constructor that it must only be
called with valid arguments, but then, I really don't see how this would be
better than having three separate constructors).
bq. this discussion also is a hint that the initial setup was not clear
Indeed, it was not obvious that the three constructors all reduced the fraction
to lowest terms. A comment would probably have been in order. Although I don't
see what this has to do with the awkward initialization of the ZERO and ONE
constants. As far as I can tell, this is simply a matter of which
method/constructor to call – converting the constructors to factory methods
should make a difference there.
In the end, I think it boils down to the question of whether one is willing to
entrust the code that converts a double value to a fraction with the
responsibility of ensuring that the calculated numerator and denominator
fulfill the conditions required by the fields' contracts (i.e. lowest terms,
sign in numerator etc.), which is not unreasonable, since the two double
constructors/methods accomplish this without code duplication. If yes, then the
code would be better of in constructors, like in the original design. If not,
then the current design seems the most adequate.
> "BigFraction(double)" is unnecessary
>
>
> Key: NUMBERS-123
> URL: https://issues.apache.org/jira/browse/NUMBERS-123
> Project: Commons Numbers
> Issue Type: Improvement
> Components: fraction
>Reporter: Gilles
>Assignee: Gilles
>Priority: Trivial
> Fix For: 1.0
>
> Attachments: NUMBERS-123__Javadoc.patch
>
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> Constructor {{BigFraction(double value)}} is only called from the
> {{from(double value)}} method.
> Actually, this constructor is misleading as it is indeed primarily a
> conversion from which appropriate {{numerator}} and {{denominator}} fields
> are computed; those could be set by
> the "direct" constructor {{BigFraction(BigInteger num, BigInteger den)}}.
> Moreover, the private field {{ZERO}} goes through this conversion code
> whereas it could constructed "directly", e.g. using {{of(0)}}. Similarly for
> field
[jira] [Commented] (NUMBERS-120) Major loss of precision in BigFraction.doubleValue() and BigFraction.floatValue()
[
https://issues.apache.org/jira/browse/NUMBERS-120?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16877281#comment-16877281
]
Heinrich Bohne commented on NUMBERS-120:
The fix in PR #63 should make the methods {{doubleValue()}} and
{{floatValue()}} _always_ calculate that {{float}}/{{double}} value that is
closer to the actual value of the fraction than any other {{float}}/{{double}}
value, rounding the result according to IEEE 754's round-to-nearest,
ties-to-even rounding mode, which is the same rounding mode used by primitive
floating-point arithmetic operations. The code is, unfortunately, a bit
convoluted, but it's the best way I could come up with to ensure maximum
precision even in "mad" corner cases like 2^54^ / (2^53^ + 1).
Also, I am not entirely convinced that I did not, at some point, re-invent the
wheel, for example with the helper method {{roundAndRightShift}}. But I don't
think the class {{BigDecimal}} would have been of much help here, because it is
designed for decimal numbers and not binary numbers, and other than that, I
don't know what other already existing code I could have used.
> Major loss of precision in BigFraction.doubleValue() and
> BigFraction.floatValue()
> -
>
> Key: NUMBERS-120
> URL: https://issues.apache.org/jira/browse/NUMBERS-120
> Project: Commons Numbers
> Issue Type: Bug
> Components: fraction
>Affects Versions: 1.0
>Reporter: Heinrich Bohne
>Priority: Minor
> Time Spent: 10m
> Remaining Estimate: 0h
>
> The method {{BigFraction.doubleValue()}} calculates the double value of
> fractions with numerators or denominators that, when converted to a
> {{double}}, round up to {{Double.POSITIVE_INFINITY}}, by right-shifting both
> the numerator and denominator synchronously until both numbers fit into 1023
> bits. Apart from the fact that the maximum number of bits an integer
> representable as a finite {{double}} can have is 1024 (an unbiased exponent
> of 1023, which is the largest possible unbiased exponent of a {{double}}
> number, means 1. ⋅ 2^1023^, which amounts to 1024 bits), this way of
> converting the fraction to a {{double}} is incredibly wasteful with precision
> if the numerator and denominator have a different bit length, because the
> smaller of the two numbers will be truncated beyond what is necessary to
> represent it as a finite {{double}}. Here is an extreme example:
> The smallest integer that rounds up to {{Double.POSITIVE_INFINITY}} when
> converted to a {{double}} is 2^1024^ - 2^970^. This is because
> {{Double.MAX_VALUE}} as an integer is a 1024-bit number with the most
> significant 53 bits set to 1 and all other 971 bits set to 0. If the 970
> least significant bits are changed in any way, the number will still round
> down to {{Double.MAX_VALUE}} as long as the 971st bit remains 0, but as soon
> as the 971st bit is set to 1, the number will round up to
> {{Double.POSITIVE_INFINITY}}.
> The smallest possible denominator greater than 1 where a single right-shift
> will cause a loss of precision is 3. 2^1024^ - 2^970^ is divisible by 3, so
> in order to create an irreducible fraction, let's add 1 to it:
> (2^1024^ - 2^970^ + 1) / 3 ≈ 5.992310449541053 ⋅ 10^307^ (which can be
> verified with {{BigDecimal}}, or, more easily, with [this online
> tool|https://www.wolframalpha.com/input/?i=(2%5E1024+-+2%5E970+%2B+1)+%2F+3].
> However, the current implementation of BigFraction.doubleValue() returns
> 8.98846567431158 ⋅ 10^307^, which differs from the correct result by a
> relative error of 50%! The same problem applies to the method
> {{BigFraction.floatValue()}}.
> This can be prevented by truncating the numerator and denominator separately,
> so that for each of the two numbers, the maximum possible precision is
> retained.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (NUMBERS-123) "BigFraction(double)" is unnecessary
[
https://issues.apache.org/jira/browse/NUMBERS-123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16876910#comment-16876910
]
Heinrich Bohne commented on NUMBERS-123:
bq. The sole purpose of the flag is efficiency
Actually, I wasn't concerned about efficiency at all when I raised this issue.
I found the design itself a bit awkward, because I think code should not do
more than it needs to do; efficiency is only a by-product of this. And because
I think that it is indeed the responsibility of the constructor (whether it is
private or public) to validate the values to which it initializes the fields, I
don't think the flag-approach is a satisfactory solution (of course, one could
include a warning in the documentation of the constructor that it must only be
called with valid arguments, but then, I really don't see how this would be
better than having three separate constructors).
bq. this discussion also is a hint that the initial setup was not clear
Indeed, it was not obvious that the three constructors all reduced the fraction
to lowest terms. A comment would probably have been in order. Although I don't
see what this has to do with the awkward initialization of the ZERO and ONE
constants. As far as I can tell, this is simply a matter of which
method/constructor to call – converting the constructors to factory methods
should make a difference there.
In the end, I think it boils down to the question of whether one is willing to
entrust the code that converts a double value to a fraction with the
responsibility of ensuring that the calculated numerator and denominator
fulfill the conditions required by the fields' contracts (i.e. lowest terms,
sign in numerator etc.), which is not unreasonable, since the two double
constructors/methods accomplish this without code duplication. If yes, then the
code would be better of in constructors, like in the original design. If not,
then the current design seems the most adequate.
> "BigFraction(double)" is unnecessary
>
>
> Key: NUMBERS-123
> URL: https://issues.apache.org/jira/browse/NUMBERS-123
> Project: Commons Numbers
> Issue Type: Improvement
> Components: fraction
>Reporter: Gilles
>Assignee: Gilles
>Priority: Trivial
> Fix For: 1.0
>
> Attachments: NUMBERS-123__Javadoc.patch
>
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> Constructor {{BigFraction(double value)}} is only called from the
> {{from(double value)}} method.
> Actually, this constructor is misleading as it is indeed primarily a
> conversion from which appropriate {{numerator}} and {{denominator}} fields
> are computed; those could be set by
> the "direct" constructor {{BigFraction(BigInteger num, BigInteger den)}}.
> Moreover, the private field {{ZERO}} goes through this conversion code
> whereas it could constructed "directly", e.g. using {{of(0)}}. Similarly for
> field {{ONE}}.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (NUMBERS-123) "BigFraction(double)" is unnecessary
[
https://issues.apache.org/jira/browse/NUMBERS-123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16876562#comment-16876562
]
Heinrich Bohne commented on NUMBERS-123:
bq. What about adding a flag to the constructor?
I also considered this possibility, but for some reason, which I'm not sure I
understand myself, but which I'll try to explain, I find the notion of a
constructor that denies (or optionally delegates) the responsibility of
ensuring the validity of the values to which it initializes the fields
alienating. And unless {{equals(Object)}} and {{hashCode()}} reduce the
numerator and denominator themselves, having the fields set to non-reduced
values breaks the class (and I guess the amount of code relying on the sign
being held by the numerator and not the denominator is much greater). After
all, the constructor is the ultimate instance that performs the initialization
in the end, and moving the validation of the arguments outside the
responsibility of the constructor makes the constructor, in a way, not a "unit"
anymore, because the correctness of its functionality, on which the integrity
of the whole object about to be created hinges, now depends on how the
constructor is invoked.
But I'm not sure if this is even relevant. Maybe the reason I find the original
design with the three constructors preferable is that, in the end, the only
functionality that was explicitly "duplicated" between
{{BigFraction(BigInteger, BigInteger)}} and the two other former constructors
was the actual initialization of the fields. No other code was duplicated
between the constructors, and still, all three constructors yielded numerators
and denominators in reduced form, with the sign being held by the numerator and
a non-zero denominator, because the three constructors were so fundamentally
different from each other.
So it somehow seems as if the constructor {{BigFraction(BigInteger,
BigInteger)}} only "won" against the other two constructors because it accepts
two arguments whose functions correspond directly to the fields that need to be
initialized. But this does not automatically mean that invoking the constructor
with the "results" of the other two former constructors makes sense, because,
as I've noted earlier, the constructor does not only do more than just
assigning the arguments to the fields, but it does nothing that has any effect
on the results of the other two former constructors.
The most consistent way of implementing such a design I could think of would be
to make a constructor that really _only_ assigns the fields, and move
everything else, _including_ the validation and reduction currently inside
{{BigFraction(BigInteger, BigInteger)}}, to static factory methods. But this
would create the problem I described in the first paragraph, so I don't think
it would be a good idea either.
> "BigFraction(double)" is unnecessary
>
>
> Key: NUMBERS-123
> URL: https://issues.apache.org/jira/browse/NUMBERS-123
> Project: Commons Numbers
> Issue Type: Improvement
> Components: fraction
>Reporter: Gilles
>Assignee: Gilles
>Priority: Trivial
> Fix For: 1.0
>
> Attachments: NUMBERS-123__Javadoc.patch
>
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> Constructor {{BigFraction(double value)}} is only called from the
> {{from(double value)}} method.
> Actually, this constructor is misleading as it is indeed primarily a
> conversion from which appropriate {{numerator}} and {{denominator}} fields
> are computed; those could be set by
> the "direct" constructor {{BigFraction(BigInteger num, BigInteger den)}}.
> Moreover, the private field {{ZERO}} goes through this conversion code
> whereas it could constructed "directly", e.g. using {{of(0)}}. Similarly for
> field {{ONE}}.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (NUMBERS-123) "BigFraction(double)" is unnecessary
[
https://issues.apache.org/jira/browse/NUMBERS-123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16876445#comment-16876445
]
Heinrich Bohne commented on NUMBERS-123:
While pondering over the impact of converting this constructor to a factory
method (and over the possible reasons for choosing one over the other in
general), I couldn't help but notice one consequence that I would consider a
drawback:
The constructor {{BigFraction(BigInteger, BigInteger)}} not only initializes
the fields {{numerator}} and {{denominator}}, but it also ascertains that the
denominator is not zero, it ensures that the sign, if present, is held by the
numerator, and it reduces the numerator and denominator to lowest terms.
All of these conditions are already fulfilled by the values passed to this
constructor from the method {{from(double)}}, even if they were enforced in a
completely different way, so subjecting the calculated numerator and
denominator to the checks and conversions of the constructor
{{BigFraction(BigInteger, BigInteger)}} is redundant. I don't think this
redundancy is outweighed by the benefits of having the code inside a static
factory method instead of a constructor (although admittedly, this is because I
don't really know what these benefits are), so actually, I find the original
design where this code was inside a constructor to be preferable (by the way,
the same also applies to the constructor converted in
[NUMBERS-124|https://issues.apache.org/jira/browse/NUMBERS-124]). What do you
think, Gilles?
> "BigFraction(double)" is unnecessary
>
>
> Key: NUMBERS-123
> URL: https://issues.apache.org/jira/browse/NUMBERS-123
> Project: Commons Numbers
> Issue Type: Improvement
> Components: fraction
>Reporter: Gilles
>Assignee: Gilles
>Priority: Trivial
> Fix For: 1.0
>
> Attachments: NUMBERS-123__Javadoc.patch
>
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> Constructor {{BigFraction(double value)}} is only called from the
> {{from(double value)}} method.
> Actually, this constructor is misleading as it is indeed primarily a
> conversion from which appropriate {{numerator}} and {{denominator}} fields
> are computed; those could be set by
> the "direct" constructor {{BigFraction(BigInteger num, BigInteger den)}}.
> Moreover, the private field {{ZERO}} goes through this conversion code
> whereas it could constructed "directly", e.g. using {{of(0)}}. Similarly for
> field {{ONE}}.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Resolved] (NUMBERS-127) Fraction(int, int) rejects possibly reducible numerator or denominator 2^-31
[ https://issues.apache.org/jira/browse/NUMBERS-127?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Heinrich Bohne resolved NUMBERS-127. Resolution: Fixed Fix Version/s: 1.0 > Fraction(int, int) rejects possibly reducible numerator or denominator 2^-31 > > > Key: NUMBERS-127 > URL: https://issues.apache.org/jira/browse/NUMBERS-127 > Project: Commons Numbers > Issue Type: Bug > Components: fraction >Affects Versions: 1.0 >Reporter: Heinrich Bohne >Priority: Minor > Fix For: 1.0 > > Time Spent: 0.5h > Remaining Estimate: 0h > > Since the sign of a fraction is held by the numerator, it is possible that > the values passed as numerator and denominator need to be negated. This is > not possible for 2^-31 due to overflow, so the constructor throws an > exception in this case. However, the constructor forgets that this value > might be reducible if the passed numerator and denominator are not coprime, > so it rejects values where the representation in lowest terms would not > overflow. > Also, the check whether the denominator is negative and the corresponding > negation of the numerator and denominator is coded twice, with the second > conditional block being dead code because the sign has already been moved to > the numerator by then. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Resolved] (NUMBERS-125) BigFraction.reduce() and Fraction.getReducedFraction(int, int) are unnecessary
[
https://issues.apache.org/jira/browse/NUMBERS-125?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Heinrich Bohne resolved NUMBERS-125.
Resolution: Fixed
Fix Version/s: 1.0
> BigFraction.reduce() and Fraction.getReducedFraction(int, int) are unnecessary
> --
>
> Key: NUMBERS-125
> URL: https://issues.apache.org/jira/browse/NUMBERS-125
> Project: Commons Numbers
> Issue Type: Improvement
> Components: fraction
>Affects Versions: 1.0
>Reporter: Heinrich Bohne
>Priority: Minor
> Fix For: 1.0
>
> Time Spent: 40m
> Remaining Estimate: 0h
>
> The instance method {{BigFraction.reduce()}} is unnecessary, because the only
> constructor in this class already reduces the fraction to lowest terms.
> Likewise, the static factory method {{Fraction.getReducedFraction(int, int)}}
> is unnecessary, because it is functionally completely equivalent to the
> factory method {{Fraction.of(int, int)}}.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Updated] (NUMBERS-127) Fraction(int, int) rejects possibly reducible numerator or denominator 2^-31
[ https://issues.apache.org/jira/browse/NUMBERS-127?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Heinrich Bohne updated NUMBERS-127: --- Description: Since the sign of a fraction is held by the numerator, it is possible that the values passed as numerator and denominator need to be negated. This is not possible for 2^-31 due to overflow, so the constructor throws an exception in this case. However, the constructor forgets that this value might be reducible if the passed numerator and denominator are not coprime, so it rejects values where the representation in lowest terms would not overflow. Also, the check whether the denominator is negative and the corresponding negation of the numerator and denominator is coded twice, with the second conditional block being dead code because the sign has already been moved to the numerator by then. was:Since the sign of a fraction is held by the numerator, it is possible that the values passed as numerator and denominator need to be negated. This is not possible for 2^-31 due to overflow, so the constructor throws an exception in this case. However, the constructor forgets that this value might be reducible if the passed numerator and denominator are not coprime, so it rejects values where the representation in lowest terms would not overflow. > Fraction(int, int) rejects possibly reducible numerator or denominator 2^-31 > > > Key: NUMBERS-127 > URL: https://issues.apache.org/jira/browse/NUMBERS-127 > Project: Commons Numbers > Issue Type: Bug > Components: fraction >Affects Versions: 1.0 >Reporter: Heinrich Bohne >Priority: Minor > > Since the sign of a fraction is held by the numerator, it is possible that > the values passed as numerator and denominator need to be negated. This is > not possible for 2^-31 due to overflow, so the constructor throws an > exception in this case. However, the constructor forgets that this value > might be reducible if the passed numerator and denominator are not coprime, > so it rejects values where the representation in lowest terms would not > overflow. > Also, the check whether the denominator is negative and the corresponding > negation of the numerator and denominator is coded twice, with the second > conditional block being dead code because the sign has already been moved to > the numerator by then. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Created] (NUMBERS-127) Fraction(int, int) rejects possibly reducible numerator or denominator 2^-31
Heinrich Bohne created NUMBERS-127: -- Summary: Fraction(int, int) rejects possibly reducible numerator or denominator 2^-31 Key: NUMBERS-127 URL: https://issues.apache.org/jira/browse/NUMBERS-127 Project: Commons Numbers Issue Type: Bug Components: fraction Affects Versions: 1.0 Reporter: Heinrich Bohne Since the sign of a fraction is held by the numerator, it is possible that the values passed as numerator and denominator need to be negated. This is not possible for 2^-31 due to overflow, so the constructor throws an exception in this case. However, the constructor forgets that this value might be reducible if the passed numerator and denominator are not coprime, so it rejects values where the representation in lowest terms would not overflow. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (NUMBERS-125) BigFraction.reduce() and Fraction.getReducedFraction(int, int) are unnecessary
[
https://issues.apache.org/jira/browse/NUMBERS-125?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16876126#comment-16876126
]
Heinrich Bohne commented on NUMBERS-125:
With {{BigFraction.reduce()}} gone, {{BigFraction.equals(Object)}} relies on
the fields {{numerator}} and {{denominator}} being reduced to lowest terms, so
this _had_ to be specified in the documentation of those two fields. The
documentation of the public API was unaffected by this.
> BigFraction.reduce() and Fraction.getReducedFraction(int, int) are unnecessary
> --
>
> Key: NUMBERS-125
> URL: https://issues.apache.org/jira/browse/NUMBERS-125
> Project: Commons Numbers
> Issue Type: Improvement
> Components: fraction
>Affects Versions: 1.0
>Reporter: Heinrich Bohne
>Priority: Minor
> Time Spent: 10m
> Remaining Estimate: 0h
>
> The instance method {{BigFraction.reduce()}} is unnecessary, because the only
> constructor in this class already reduces the fraction to lowest terms.
> Likewise, the static factory method {{Fraction.getReducedFraction(int, int)}}
> is unnecessary, because it is functionally completely equivalent to the
> factory method {{Fraction.of(int, int)}}.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Created] (NUMBERS-125) BigFraction.reduce() and Fraction.getReducedFraction(int, int) are unnecessary
Heinrich Bohne created NUMBERS-125:
--
Summary: BigFraction.reduce() and Fraction.getReducedFraction(int,
int) are unnecessary
Key: NUMBERS-125
URL: https://issues.apache.org/jira/browse/NUMBERS-125
Project: Commons Numbers
Issue Type: Improvement
Components: fraction
Affects Versions: 1.0
Reporter: Heinrich Bohne
The instance method {{BigFraction.reduce()}} is unnecessary, because the only
constructor in this class already reduces the fraction to lowest terms.
Likewise, the static factory method {{Fraction.getReducedFraction(int, int)}}
is unnecessary, because it is functionally completely equivalent to the factory
method {{Fraction.of(int, int)}}.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Updated] (NUMBERS-123) "BigFraction(double)" is unnecessary
[
https://issues.apache.org/jira/browse/NUMBERS-123?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Heinrich Bohne updated NUMBERS-123:
---
Attachment: NUMBERS-123__Javadoc.patch
> "BigFraction(double)" is unnecessary
>
>
> Key: NUMBERS-123
> URL: https://issues.apache.org/jira/browse/NUMBERS-123
> Project: Commons Numbers
> Issue Type: Improvement
> Components: fraction
>Reporter: Gilles
>Assignee: Gilles
>Priority: Trivial
> Fix For: 1.0
>
> Attachments: NUMBERS-123__Javadoc.patch
>
> Time Spent: 10m
> Remaining Estimate: 0h
>
> Constructor {{BigFraction(double value)}} is only called from the
> {{from(double value)}} method.
> Actually, this constructor is misleading as it is indeed primarily a
> conversion from which appropriate {{numerator}} and {{denominator}} fields
> are computed; those could be set by
> the "direct" constructor {{BigFraction(BigInteger num, BigInteger den)}}.
> Moreover, the private field {{ZERO}} goes through this conversion code
> whereas it could constructed "directly", e.g. using {{of(0)}}. Similarly for
> field {{ONE}}.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (NUMBERS-123) "BigFraction(double)" is unnecessary
[
https://issues.apache.org/jira/browse/NUMBERS-123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16875890#comment-16875890
]
Heinrich Bohne commented on NUMBERS-123:
The Javadoc still needs to be updated in two places, here's a patch.
[^NUMBERS-123__Javadoc.patch]
> "BigFraction(double)" is unnecessary
>
>
> Key: NUMBERS-123
> URL: https://issues.apache.org/jira/browse/NUMBERS-123
> Project: Commons Numbers
> Issue Type: Improvement
> Components: fraction
>Reporter: Gilles
>Assignee: Gilles
>Priority: Trivial
> Fix For: 1.0
>
> Attachments: NUMBERS-123__Javadoc.patch
>
> Time Spent: 10m
> Remaining Estimate: 0h
>
> Constructor {{BigFraction(double value)}} is only called from the
> {{from(double value)}} method.
> Actually, this constructor is misleading as it is indeed primarily a
> conversion from which appropriate {{numerator}} and {{denominator}} fields
> are computed; those could be set by
> the "direct" constructor {{BigFraction(BigInteger num, BigInteger den)}}.
> Moreover, the private field {{ZERO}} goes through this conversion code
> whereas it could constructed "directly", e.g. using {{of(0)}}. Similarly for
> field {{ONE}}.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (NUMBERS-119) BigFraction(double) constructor does not treat subnormal numbers correctly
[
https://issues.apache.org/jira/browse/NUMBERS-119?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16875888#comment-16875888
]
Heinrich Bohne commented on NUMBERS-119:
Excellent, thanks! Regarding NUMBERS-123 and 124: I found one missed Javadoc
update, for which I'll create a pull request. Or is a patch preferable for such
trivial changes? I'll create a patch too, if I manage to upload it in
NUMBERS-123.
> BigFraction(double) constructor does not treat subnormal numbers correctly
> --
>
> Key: NUMBERS-119
> URL: https://issues.apache.org/jira/browse/NUMBERS-119
> Project: Commons Numbers
> Issue Type: Bug
> Components: fraction
>Affects Versions: 1.0
>Reporter: Heinrich Bohne
>Priority: Minor
> Fix For: 1.0
>
> Time Spent: 1h 20m
> Remaining Estimate: 0h
>
> The constructor {{BigFraction(double)}} does not take into account the fact
> that, when the biased exponent of a {{double}} value is {{0}} and the
> mantissa is not {{0}} (i.e. when the value represents a subnormal number),
> the actual exponent in effect is not {{-1023}} but {{-1022}} (or, in other
> words, the effective exponent bias is not {{1023}} but {{1022}}).
> The value of the created {{BigFraction}} is therefore not equal to the value
> of the passed {{double}} argument.
> Also, since the constructor does not handle the case of zero separately, it
> creates a fraction with a numerator of 0 and a denominator of 2^1075^, which
> is not very memory-efficient.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Resolved] (NUMBERS-119) BigFraction(double) constructor does not treat subnormal numbers correctly
[
https://issues.apache.org/jira/browse/NUMBERS-119?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Heinrich Bohne resolved NUMBERS-119.
Resolution: Fixed
Fix Version/s: 1.0
> BigFraction(double) constructor does not treat subnormal numbers correctly
> --
>
> Key: NUMBERS-119
> URL: https://issues.apache.org/jira/browse/NUMBERS-119
> Project: Commons Numbers
> Issue Type: Bug
> Components: fraction
>Affects Versions: 1.0
>Reporter: Heinrich Bohne
>Priority: Minor
> Fix For: 1.0
>
> Time Spent: 1h 20m
> Remaining Estimate: 0h
>
> The constructor {{BigFraction(double)}} does not take into account the fact
> that, when the biased exponent of a {{double}} value is {{0}} and the
> mantissa is not {{0}} (i.e. when the value represents a subnormal number),
> the actual exponent in effect is not {{-1023}} but {{-1022}} (or, in other
> words, the effective exponent bias is not {{1023}} but {{1022}}).
> The value of the created {{BigFraction}} is therefore not equal to the value
> of the passed {{double}} argument.
> Also, since the constructor does not handle the case of zero separately, it
> creates a fraction with a numerator of 0 and a denominator of 2^1075^, which
> is not very memory-efficient.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (NUMBERS-119) BigFraction(double) constructor does not treat subnormal numbers correctly
[
https://issues.apache.org/jira/browse/NUMBERS-119?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16875196#comment-16875196
]
Heinrich Bohne commented on NUMBERS-119:
So, PR #56 has been out there for a while. Is it fit to be merged? Or is there
something that should be changed about it?
> BigFraction(double) constructor does not treat subnormal numbers correctly
> --
>
> Key: NUMBERS-119
> URL: https://issues.apache.org/jira/browse/NUMBERS-119
> Project: Commons Numbers
> Issue Type: Bug
> Components: fraction
>Affects Versions: 1.0
>Reporter: Heinrich Bohne
>Priority: Minor
> Time Spent: 1h 10m
> Remaining Estimate: 0h
>
> The constructor {{BigFraction(double)}} does not take into account the fact
> that, when the biased exponent of a {{double}} value is {{0}} and the
> mantissa is not {{0}} (i.e. when the value represents a subnormal number),
> the actual exponent in effect is not {{-1023}} but {{-1022}} (or, in other
> words, the effective exponent bias is not {{1023}} but {{1022}}).
> The value of the created {{BigFraction}} is therefore not equal to the value
> of the passed {{double}} argument.
> Also, since the constructor does not handle the case of zero separately, it
> creates a fraction with a numerator of 0 and a denominator of 2^1075^, which
> is not very memory-efficient.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (NUMBERS-119) BigFraction(double) constructor does not treat subnormal numbers correctly
[
https://issues.apache.org/jira/browse/NUMBERS-119?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16874441#comment-16874441
]
Heinrich Bohne commented on NUMBERS-119:
I've merged the master branch into PR #56. For some reason, this made the false
reports by Coveralls about uncovered lines in {{ArithmeticUtils}} go away. Now,
according to the report, the only line previously covered but now uncovered is
the line in {{reduce()}} that becomes unreachable after this change because the
numerator and denominator are now _always_ reduced to lowest terms, meaning
that line cannot possibly be covered by unit tests. But deleting the line would
be illegal unless the contract of the class were changed as well, because the
class' contract doesn't require the numerator and denominator to be reduced to
lowest terms, and changing the class' contract in this way seems to be outside
the scope of this ticket.
> BigFraction(double) constructor does not treat subnormal numbers correctly
> --
>
> Key: NUMBERS-119
> URL: https://issues.apache.org/jira/browse/NUMBERS-119
> Project: Commons Numbers
> Issue Type: Bug
> Components: fraction
>Affects Versions: 1.0
>Reporter: Heinrich Bohne
>Priority: Minor
> Time Spent: 1h 10m
> Remaining Estimate: 0h
>
> The constructor {{BigFraction(double)}} does not take into account the fact
> that, when the biased exponent of a {{double}} value is {{0}} and the
> mantissa is not {{0}} (i.e. when the value represents a subnormal number),
> the actual exponent in effect is not {{-1023}} but {{-1022}} (or, in other
> words, the effective exponent bias is not {{1023}} but {{1022}}).
> The value of the created {{BigFraction}} is therefore not equal to the value
> of the passed {{double}} argument.
> Also, since the constructor does not handle the case of zero separately, it
> creates a fraction with a numerator of 0 and a denominator of 2^1075^, which
> is not very memory-efficient.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Resolved] (NUMBERS-122) Helper assertion methods in BigFractionTest not strict enough
[
https://issues.apache.org/jira/browse/NUMBERS-122?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Heinrich Bohne resolved NUMBERS-122.
Resolution: Fixed
Fix Version/s: 1.0
> Helper assertion methods in BigFractionTest not strict enough
> -
>
> Key: NUMBERS-122
> URL: https://issues.apache.org/jira/browse/NUMBERS-122
> Project: Commons Numbers
> Issue Type: Bug
> Components: fraction
>Affects Versions: 1.0
>Reporter: Heinrich Bohne
>Priority: Minor
> Fix For: 1.0
>
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> The helper methods {{assertFraction(int, int, BigFraction)}} and
> {{assertFraction(long, long, BigFraction)}} don't check whether the numerator
> and denominator of the {{BigFraction}} are equivalent to the first two
> arguments. Rather, they check whether the numerator and denominator
> _truncated to an {{int}} or {{long}}_ are equal to the first two arguments.
> This is unsatisfactory and also probably not in the spirit of the creator of
> these methods.
> Also, the method {{assertFraction(int, int, BigFraction)}} is redundant,
> because an {{int}} can be cast to a {{long}} without loss of precision, so
> {{assertFraction(long, long, BigFraction)}} is sufficient to handle both
> {{int}} and {{long}} arguments.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Resolved] (NUMBERS-121) Deprecated HTML tag in BigFraction
[
https://issues.apache.org/jira/browse/NUMBERS-121?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Heinrich Bohne resolved NUMBERS-121.
Resolution: Fixed
Fix Version/s: 1.0
> Deprecated HTML tag in BigFraction
> --
>
> Key: NUMBERS-121
> URL: https://issues.apache.org/jira/browse/NUMBERS-121
> Project: Commons Numbers
> Issue Type: Bug
> Components: fraction
>Affects Versions: 1.0
>Reporter: Heinrich Bohne
>Priority: Minor
> Fix For: 1.0
>
> Time Spent: 1h 40m
> Remaining Estimate: 0h
>
> The deprecated HTML tag {{}} occurs multiple times in the Javadoc of the
> class {{BigFraction}}, which causes the execution of the Maven Javadoc Plugin
> to fail.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Updated] (NUMBERS-122) Helper assertion methods in BigFractionTest not strict enough
[
https://issues.apache.org/jira/browse/NUMBERS-122?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Heinrich Bohne updated NUMBERS-122:
---
Description:
The helper methods {{assertFraction(int, int, BigFraction)}} and
{{assertFraction(long, long, BigFraction)}} don't check whether the numerator
and denominator of the {{BigFraction}} are equivalent to the first two
arguments. Rather, they check whether the numerator and denominator _truncated
to an {{int}} or {{long}}_ are equal to the first two arguments. This is
unsatisfactory and also probably not in the spirit of the creator of these
methods.
Also, the method {{assertFraction(int, int, BigFraction)}} is redundant,
because an {{int}} can be cast to a {{long}} without loss of precision, so
{{assertFraction(long, long, BigFraction)}} is sufficient to handle both
{{int}} and {{long}} arguments.
was:The helper methods {{assertFraction(int, int, BigFraction)}} and
{{assertFraction(long, long, BigFraction)}} don't check whether the numerator
and denominator of the {{BigFraction}} are equivalent to the first two
arguments. Rather, they check whether the numerator and denominator _truncated
to an {{int}} or {{long}}_ are equal to the first two arguments. This is
unsatisfactory and also probably not in the spirit of the creator of these
methods.
> Helper assertion methods in BigFractionTest not strict enough
> -
>
> Key: NUMBERS-122
> URL: https://issues.apache.org/jira/browse/NUMBERS-122
> Project: Commons Numbers
> Issue Type: Bug
> Components: fraction
>Affects Versions: 1.0
>Reporter: Heinrich Bohne
>Priority: Minor
>
> The helper methods {{assertFraction(int, int, BigFraction)}} and
> {{assertFraction(long, long, BigFraction)}} don't check whether the numerator
> and denominator of the {{BigFraction}} are equivalent to the first two
> arguments. Rather, they check whether the numerator and denominator
> _truncated to an {{int}} or {{long}}_ are equal to the first two arguments.
> This is unsatisfactory and also probably not in the spirit of the creator of
> these methods.
> Also, the method {{assertFraction(int, int, BigFraction)}} is redundant,
> because an {{int}} can be cast to a {{long}} without loss of precision, so
> {{assertFraction(long, long, BigFraction)}} is sufficient to handle both
> {{int}} and {{long}} arguments.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Created] (NUMBERS-122) Helper assertion methods in BigFractionTest not strict enough
Heinrich Bohne created NUMBERS-122:
--
Summary: Helper assertion methods in BigFractionTest not strict
enough
Key: NUMBERS-122
URL: https://issues.apache.org/jira/browse/NUMBERS-122
Project: Commons Numbers
Issue Type: Bug
Components: fraction
Affects Versions: 1.0
Reporter: Heinrich Bohne
The helper methods {{assertFraction(int, int, BigFraction)}} and
{{assertFraction(long, long, BigFraction)}} don't check whether the numerator
and denominator of the {{BigFraction}} are equivalent to the first two
arguments. Rather, they check whether the numerator and denominator _truncated
to an {{int}} or {{long}}_ are equal to the first two arguments. This is
unsatisfactory and also probably not in the spirit of the creator of these
methods.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (NUMBERS-121) Deprecated HTML tag in BigFraction
[
https://issues.apache.org/jira/browse/NUMBERS-121?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16873998#comment-16873998
]
Heinrich Bohne commented on NUMBERS-121:
{quote}You can run the jacoco plugin to get the report:
mvn test jacoco:report{quote}
Excellent, thanks.
> Deprecated HTML tag in BigFraction
> --
>
> Key: NUMBERS-121
> URL: https://issues.apache.org/jira/browse/NUMBERS-121
> Project: Commons Numbers
> Issue Type: Bug
> Components: fraction
>Affects Versions: 1.0
>Reporter: Heinrich Bohne
>Priority: Minor
> Time Spent: 50m
> Remaining Estimate: 0h
>
> The deprecated HTML tag {{}} occurs multiple times in the Javadoc of the
> class {{BigFraction}}, which causes the execution of the Maven Javadoc Plugin
> to fail.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (NUMBERS-121) Deprecated HTML tag in BigFraction
[
https://issues.apache.org/jira/browse/NUMBERS-121?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16873944#comment-16873944
]
Heinrich Bohne commented on NUMBERS-121:
So is PR #58 not in order? I would be in favor of getting rid of these obsolete
html tags, because having to manually fix this in my local copy just to check
coverage by running {{mvn clean site site:stage}} is getting tiresome. Or is
there a way to check coverage without running the javadoc plugin?
> Deprecated HTML tag in BigFraction
> --
>
> Key: NUMBERS-121
> URL: https://issues.apache.org/jira/browse/NUMBERS-121
> Project: Commons Numbers
> Issue Type: Bug
> Components: fraction
>Affects Versions: 1.0
>Reporter: Heinrich Bohne
>Priority: Minor
> Time Spent: 50m
> Remaining Estimate: 0h
>
> The deprecated HTML tag {{}} occurs multiple times in the Javadoc of the
> class {{BigFraction}}, which causes the execution of the Maven Javadoc Plugin
> to fail.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (NUMBERS-121) Deprecated HTML tag in BigFraction
[
https://issues.apache.org/jira/browse/NUMBERS-121?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16873679#comment-16873679
]
Heinrich Bohne commented on NUMBERS-121:
Interesting, I didn't know that Maven can be set up to render LaTeX commands in
Javadoc. I used the \mathit command, which seems to be the standard way of
rendering multi-letter variable names, from what I found on the Internet.
> Deprecated HTML tag in BigFraction
> --
>
> Key: NUMBERS-121
> URL: https://issues.apache.org/jira/browse/NUMBERS-121
> Project: Commons Numbers
> Issue Type: Bug
> Components: fraction
>Affects Versions: 1.0
>Reporter: Heinrich Bohne
>Priority: Minor
> Time Spent: 40m
> Remaining Estimate: 0h
>
> The deprecated HTML tag {{}} occurs multiple times in the Javadoc of the
> class {{BigFraction}}, which causes the execution of the Maven Javadoc Plugin
> to fail.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Created] (NUMBERS-121) Deprecated HTML tag in BigFraction
Heinrich Bohne created NUMBERS-121:
--
Summary: Deprecated HTML tag in BigFraction
Key: NUMBERS-121
URL: https://issues.apache.org/jira/browse/NUMBERS-121
Project: Commons Numbers
Issue Type: Bug
Components: fraction
Affects Versions: 1.0
Reporter: Heinrich Bohne
The deprecated HTML tag {{}} occurs multiple times in the Javadoc of the
class {{BigFraction}}, which causes the execution of the Maven Javadoc Plugin
to fail.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Updated] (NUMBERS-119) BigFraction(double) constructor does not treat subnormal numbers correctly
[
https://issues.apache.org/jira/browse/NUMBERS-119?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Heinrich Bohne updated NUMBERS-119:
---
Description:
The constructor {{BigFraction(double)}} does not take into account the fact
that, when the biased exponent of a {{double}} value is {{0}} and the mantissa
is not {{0}} (i.e. when the value represents a subnormal number), the actual
exponent in effect is not {{-1023}} but {{-1022}} (or, in other words, the
effective exponent bias is not {{1023}} but {{1022}}).
The value of the created {{BigFraction}} is therefore not equal to the value of
the passed {{double}} argument.
Also, since the constructor does not handle the case of zero separately, it
creates a fraction with a numerator of 0 and a denominator of 2^1075^, which
is not very memory-efficient.
was:
The constructor {{BigFraction(double)}} does not take into account the fact
that, when the biased exponent of a {{double}} value is {{0}} and the mantissa
is not {{0}} (i.e. when the value represents a subnormal number), the actual
exponent in effect is not {{-1023}} but {{-1022}} (or, in other words, the
effective exponent bias is not {{1023}} but {{1022}}).
The value of the created {{BigFraction}} is therefore not equal to the value of
the passed {{double}} argument.
> BigFraction(double) constructor does not treat subnormal numbers correctly
> --
>
> Key: NUMBERS-119
> URL: https://issues.apache.org/jira/browse/NUMBERS-119
> Project: Commons Numbers
> Issue Type: Bug
> Components: fraction
>Affects Versions: 1.0
>Reporter: Heinrich Bohne
>Priority: Minor
> Time Spent: 1h
> Remaining Estimate: 0h
>
> The constructor {{BigFraction(double)}} does not take into account the fact
> that, when the biased exponent of a {{double}} value is {{0}} and the
> mantissa is not {{0}} (i.e. when the value represents a subnormal number),
> the actual exponent in effect is not {{-1023}} but {{-1022}} (or, in other
> words, the effective exponent bias is not {{1023}} but {{1022}}).
> The value of the created {{BigFraction}} is therefore not equal to the value
> of the passed {{double}} argument.
> Also, since the constructor does not handle the case of zero separately, it
> creates a fraction with a numerator of 0 and a denominator of 2^1075^, which
> is not very memory-efficient.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (NUMBERS-119) BigFraction(double) constructor does not treat subnormal numbers correctly
[
https://issues.apache.org/jira/browse/NUMBERS-119?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16872173#comment-16872173
]
Heinrich Bohne commented on NUMBERS-119:
About the code coverage decrease in the method {{reduce()}} caused by PR #56
(the coverage decrease in {{ArithmeticUtils}} is a myth invented by Coveralls,
my IDE even shows that the condition in line 112 is always false):
Since the local variable {{k}} had to be set inside the conditional tree that
depends on the value of the exponent rather than outside it as was previously
the case, I thought I would also make a special case for zero, because it
doesn't make sense for the denominator of a zero-fraction to be 2^1074^. It
didn't occur to me that this would have an effect on the code coverage.
However, with this change, the uncovered line in the method {{reduce()}}
becomes unreachable, because the zero fraction created from this double
constructor was the only case where the fraction was not already reduced to
lowest terms. This renders the {{reduce()}} method obsolete anyway, which could
be the target of a new JIRA ticket.
> BigFraction(double) constructor does not treat subnormal numbers correctly
> --
>
> Key: NUMBERS-119
> URL: https://issues.apache.org/jira/browse/NUMBERS-119
> Project: Commons Numbers
> Issue Type: Bug
> Components: fraction
>Affects Versions: 1.0
>Reporter: Heinrich Bohne
>Priority: Minor
> Time Spent: 1h
> Remaining Estimate: 0h
>
> The constructor {{BigFraction(double)}} does not take into account the fact
> that, when the biased exponent of a {{double}} value is {{0}} and the
> mantissa is not {{0}} (i.e. when the value represents a subnormal number),
> the actual exponent in effect is not {{-1023}} but {{-1022}} (or, in other
> words, the effective exponent bias is not {{1023}} but {{1022}}).
> The value of the created {{BigFraction}} is therefore not equal to the value
> of the passed {{double}} argument.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (NUMBERS-120) Major loss of precision in BigFraction.doubleValue() and BigFraction.floatValue()
[
https://issues.apache.org/jira/browse/NUMBERS-120?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16871762#comment-16871762
]
Heinrich Bohne commented on NUMBERS-120:
In fact, even converting the numerator and denominator to a {{double}}
separately is not sufficiently precise. For example, consider the following
irreducible fraction:
2^54^ / (2^53^ + 1)
The numerator and denominator round to 2^54^ and 2^53^ when converted to a
double (due to the round-to-even rule), which would produce a result of {{2.0}}
when divided. However, the [exact value of the
fraction|https://www.wolframalpha.com/input/?i=(2%5E54)%2F(2%5E53%2B1)+to+base+2]
is closer to 2 - 2^-52^ (which is representable exactly as a {{double}}) than
it is to 2, so dividing the {{doubleValue()}} of the numerator by the
{{doubleValue()}} of the denominator is not acceptable.
> Major loss of precision in BigFraction.doubleValue() and
> BigFraction.floatValue()
> -
>
> Key: NUMBERS-120
> URL: https://issues.apache.org/jira/browse/NUMBERS-120
> Project: Commons Numbers
> Issue Type: Bug
> Components: fraction
>Affects Versions: 1.0
>Reporter: Heinrich Bohne
>Priority: Minor
>
> The method {{BigFraction.doubleValue()}} calculates the double value of
> fractions with numerators or denominators that, when converted to a
> {{double}}, round up to {{Double.POSITIVE_INFINITY}}, by right-shifting both
> the numerator and denominator synchronously until both numbers fit into 1023
> bits. Apart from the fact that the maximum number of bits an integer
> representable as a finite {{double}} can have is 1024 (an unbiased exponent
> of 1023, which is the largest possible unbiased exponent of a {{double}}
> number, means 1. ⋅ 2^1023^, which amounts to 1024 bits), this way of
> converting the fraction to a {{double}} is incredibly wasteful with precision
> if the numerator and denominator have a different bit length, because the
> smaller of the two numbers will be truncated beyond what is necessary to
> represent it as a finite {{double}}. Here is an extreme example:
> The smallest integer that rounds up to {{Double.POSITIVE_INFINITY}} when
> converted to a {{double}} is 2^1024^ - 2^970^. This is because
> {{Double.MAX_VALUE}} as an integer is a 1024-bit number with the most
> significant 53 bits set to 1 and all other 971 bits set to 0. If the 970
> least significant bits are changed in any way, the number will still round
> down to {{Double.MAX_VALUE}} as long as the 971st bit remains 0, but as soon
> as the 971st bit is set to 1, the number will round up to
> {{Double.POSITIVE_INFINITY}}.
> The smallest possible denominator greater than 1 where a single right-shift
> will cause a loss of precision is 3. 2^1024^ - 2^970^ is divisible by 3, so
> in order to create an irreducible fraction, let's add 1 to it:
> (2^1024^ - 2^970^ + 1) / 3 ≈ 5.992310449541053 ⋅ 10^307^ (which can be
> verified with {{BigDecimal}}, or, more easily, with [this online
> tool|https://www.wolframalpha.com/input/?i=(2%5E1024+-+2%5E970+%2B+1)+%2F+3].
> However, the current implementation of BigFraction.doubleValue() returns
> 8.98846567431158 ⋅ 10^307^, which differs from the correct result by a
> relative error of 50%! The same problem applies to the method
> {{BigFraction.floatValue()}}.
> This can be prevented by truncating the numerator and denominator separately,
> so that for each of the two numbers, the maximum possible precision is
> retained.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Created] (NUMBERS-120) Major loss of precision in BigFraction.doubleValue() and BigFraction.floatValue()
Heinrich Bohne created NUMBERS-120:
--
Summary: Major loss of precision in BigFraction.doubleValue() and
BigFraction.floatValue()
Key: NUMBERS-120
URL: https://issues.apache.org/jira/browse/NUMBERS-120
Project: Commons Numbers
Issue Type: Bug
Components: fraction
Affects Versions: 1.0
Reporter: Heinrich Bohne
The method {{BigFraction.doubleValue()}} calculates the double value of
fractions with numerators or denominators that, when converted to a {{double}},
round up to {{Double.POSITIVE_INFINITY}}, by right-shifting both the numerator
and denominator synchronously until both numbers fit into 1023 bits. Apart from
the fact that the maximum number of bits an integer representable as a finite
{{double}} can have is 1024 (an unbiased exponent of 1023, which is the largest
possible unbiased exponent of a {{double}} number, means 1. ⋅ 2^1023^,
which amounts to 1024 bits), this way of converting the fraction to a
{{double}} is incredibly wasteful with precision if the numerator and
denominator have a different bit length, because the smaller of the two numbers
will be truncated beyond what is necessary to represent it as a finite
{{double}}. Here is an extreme example:
The smallest integer that rounds up to {{Double.POSITIVE_INFINITY}} when
converted to a {{double}} is 2^1024^ - 2^970^. This is because
{{Double.MAX_VALUE}} as an integer is a 1024-bit number with the most
significant 53 bits set to 1 and all other 971 bits set to 0. If the 970 least
significant bits are changed in any way, the number will still round down to
{{Double.MAX_VALUE}} as long as the 971st bit remains 0, but as soon as the
971st bit is set to 1, the number will round up to {{Double.POSITIVE_INFINITY}}.
The smallest possible denominator greater than 1 where a single right-shift
will cause a loss of precision is 3. 2^1024^ - 2^970^ is divisible by 3, so
in order to create an irreducible fraction, let's add 1 to it:
(2^1024^ - 2^970^ + 1) / 3 ≈ 5.992310449541053 ⋅ 10^307^ (which can be
verified with {{BigDecimal}}, or, more easily, with [this online
tool|https://www.wolframalpha.com/input/?i=(2%5E1024+-+2%5E970+%2B+1)+%2F+3].
However, the current implementation of BigFraction.doubleValue() returns
8.98846567431158 ⋅ 10^307^, which differs from the correct result by a
relative error of 50%! The same problem applies to the method
{{BigFraction.floatValue()}}.
This can be prevented by truncating the numerator and denominator separately,
so that for each of the two numbers, the maximum possible precision is retained.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Resolved] (NUMBERS-118) Reduce code duplication between BigFractionTest and FractionTest
[
https://issues.apache.org/jira/browse/NUMBERS-118?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Heinrich Bohne resolved NUMBERS-118.
Resolution: Fixed
Fix Version/s: 1.0
> Reduce code duplication between BigFractionTest and FractionTest
>
>
> Key: NUMBERS-118
> URL: https://issues.apache.org/jira/browse/NUMBERS-118
> Project: Commons Numbers
> Issue Type: Improvement
> Components: fraction
>Affects Versions: 1.0
>Reporter: Heinrich Bohne
>Priority: Minor
> Fix For: 1.0
>
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> Apparently, the class {{BigFractionTest}} has been created mainly by
> copy-pasting {{FractionTest}}, resulting in a lot of duplicate test cases.
> This code duplication can be mitigated by extracting some of the test cases
> that are used by both classes to a single location.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Resolved] (NUMBERS-116) Remove redundant methods in ArithmeticUtils
[
https://issues.apache.org/jira/browse/NUMBERS-116?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Heinrich Bohne resolved NUMBERS-116.
Resolution: Fixed
Fix Version/s: 1.0
> Remove redundant methods in ArithmeticUtils
> ---
>
> Key: NUMBERS-116
> URL: https://issues.apache.org/jira/browse/NUMBERS-116
> Project: Commons Numbers
> Issue Type: Improvement
> Components: core
>Reporter: Heinrich Bohne
>Priority: Minor
> Fix For: 1.0
>
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> As has been
> [discussed|http://mail-archives.apache.org/mod_mbox/commons-dev/201906.mbox/%3C940f9ff0-0b25-cd31-ddb3-a95ca777ba06%40gmx.at%3E]
> on the developers' mailing list, the following methods from the class
> {{ArithmeticUtils}} can be removed:
> {{addAndCheck(int, int)}}
> {{addAndCheck(long, long)}}
> {{mulAndCheck(int, int)}}
> {{mulAndCheck(long, long)}}
> {{subAndCheck(int, int)}}
> {{subAndCheck(long, long)}}
> And their usages replaced with the following equivalent methods from
> {{java.lang.Math}}:
> {{addExact(int, int)}}
> {{addExact(long, long)}}
> {{multiplyExact(int, int)}}
> {{multiplyExact(long, long)}}
> {{subtractExact(int, int)}}
> {{subtractExact(long, long)}}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Created] (NUMBERS-119) BigFraction(double) constructor does not treat subnormal numbers correctly
Heinrich Bohne created NUMBERS-119:
--
Summary: BigFraction(double) constructor does not treat subnormal
numbers correctly
Key: NUMBERS-119
URL: https://issues.apache.org/jira/browse/NUMBERS-119
Project: Commons Numbers
Issue Type: Bug
Components: fraction
Affects Versions: 1.0
Reporter: Heinrich Bohne
The constructor {{BigFraction(double)}} does not take into account the fact
that, when the biased exponent of a {{double}} value is {{0}} and the mantissa
is not {{0}} (i.e. when the value represents a subnormal number), the actual
exponent in effect is not {{-1023}} but {{-1022}} (or, in other words, the
effective exponent bias is not {{1023}} but {{1022}}).
The value of the created {{BigFraction}} is therefore not equal to the value of
the passed {{double}} argument.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Created] (NUMBERS-118) Reduce code duplication between BigFractionTest and FractionTest
Heinrich Bohne created NUMBERS-118:
--
Summary: Reduce code duplication between BigFractionTest and
FractionTest
Key: NUMBERS-118
URL: https://issues.apache.org/jira/browse/NUMBERS-118
Project: Commons Numbers
Issue Type: Improvement
Components: fraction
Affects Versions: 1.0
Reporter: Heinrich Bohne
Apparently, the class {{BigFractionTest}} has been created mainly by
copy-pasting {{FractionTest}}, resulting in a lot of duplicate test cases. This
code duplication can be mitigated by extracting some of the test cases that are
used by both classes to a single location.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Reopened] (MATH-1492) Replace usages of commons-numbers-core methods with equivalent methods from java.lang.Math
[
https://issues.apache.org/jira/browse/MATH-1492?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Heinrich Bohne reopened MATH-1492:
--
> Replace usages of commons-numbers-core methods with equivalent methods from
> java.lang.Math
> --
>
> Key: MATH-1492
> URL: https://issues.apache.org/jira/browse/MATH-1492
> Project: Commons Math
> Issue Type: Task
>Reporter: Heinrich Bohne
>Priority: Minor
> Fix For: 4.0
>
> Time Spent: 20m
> Remaining Estimate: 0h
>
> The usages of the following methods from
> {{org.apache.commons.numbers.core.ArithmeticUtils}}:
> {{addAndCheck(int, int)}}
> {{addAndCheck(long, long)}}
> {{mulAndCheck(int, int)}}
> {{mulAndCheck(long, long)}}
> {{subAndCheck(int, int)}}
> {{subAndCheck(long, long)}}
> Can be replaced with the following equivalent methods from {{java.lang.Math}}:
> {{addExact(int, int)}}
> {{addExact(long, long)}}
> {{multiplyExact(int, int)}}
> {{multiplyExact(long, long)}}
> {{subtractExact(int, int)}}
> {{subtractExact(long, long)}}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Resolved] (MATH-1492) Replace usages of commons-numbers-core methods with equivalent methods from java.lang.Math
[
https://issues.apache.org/jira/browse/MATH-1492?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Heinrich Bohne resolved MATH-1492.
--
Resolution: Fixed
> Replace usages of commons-numbers-core methods with equivalent methods from
> java.lang.Math
> --
>
> Key: MATH-1492
> URL: https://issues.apache.org/jira/browse/MATH-1492
> Project: Commons Math
> Issue Type: Task
>Reporter: Heinrich Bohne
>Priority: Minor
> Fix For: 4.0
>
> Time Spent: 20m
> Remaining Estimate: 0h
>
> The usages of the following methods from
> {{org.apache.commons.numbers.core.ArithmeticUtils}}:
> {{addAndCheck(int, int)}}
> {{addAndCheck(long, long)}}
> {{mulAndCheck(int, int)}}
> {{mulAndCheck(long, long)}}
> {{subAndCheck(int, int)}}
> {{subAndCheck(long, long)}}
> Can be replaced with the following equivalent methods from {{java.lang.Math}}:
> {{addExact(int, int)}}
> {{addExact(long, long)}}
> {{multiplyExact(int, int)}}
> {{multiplyExact(long, long)}}
> {{subtractExact(int, int)}}
> {{subtractExact(long, long)}}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Resolved] (NUMBERS-117) Redundant methods in several TestUtils classes
[
https://issues.apache.org/jira/browse/NUMBERS-117?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Heinrich Bohne resolved NUMBERS-117.
Resolution: Fixed
Fix Version/s: 1.0
> Redundant methods in several TestUtils classes
> --
>
> Key: NUMBERS-117
> URL: https://issues.apache.org/jira/browse/NUMBERS-117
> Project: Commons Numbers
> Issue Type: Improvement
>Reporter: Heinrich Bohne
>Priority: Minor
> Fix For: 1.0
>
> Time Spent: 20m
> Remaining Estimate: 0h
>
> The following methods from the class {{TestUtils}} in the module
> _commons-numbers-core_ are redundant:
> |{{assertEquals(double, double, double)}}|{{Assertions.assertEquals(double,
> double, double)}} already considers two {{NaN}} values equal, so the two
> methods are equivalent.|
> |{{assertEquals(String, double, double, double)}}|Equivalent to
> {{Assertions.assertEquals(double, double, double, String)}}, as explained
> above|
> |{{assertSame(double, double)}}|Equivalent to
> {{Assertions.assertEquals(double, double)}}|
> |{{assertEquals(double[], double[], double)}}|Equivalent to
> {{Assertions.assertArrayEquals(double[], double[], double)}}|
> |{{assertEquals(String, double[], double[], double)}}|Equivalent to
> {{Assertions.assertArrayEquals(double[], double[], double, String)}}|
> |{{assertEquals(String, float[], float[], float)}}|Equivalent to
> {{Assertions.assertArrayEquals(float[], float[], float, String)}}|
> Similarly, the following methods from the class {{TestUtils}} in the module
> _commons-numbers-complex-streams_ are redundant:
> |{{assertEquals(double, double, double)}}|Equivalent to
> {{Assertions.assertEquals(double, double, double)}}, as explained above|
> |{{assertEquals(String, double, double, double)}}|Equivalent to
> {{Assertions.assertEquals(double, double, double, String)}}, as explained
> above|
> |{{assertSame(double[], double[])}}|Equivalent to
> {{Assertions.assertArrayEquals(double[], double[])}}|
> |{{assertSame(float[], float[])}}|Equivalent to
> {{Assertions.assertArrayEquals(float[], float[])}}|
> |{{assertSame(double, double)}}|Equivalent to
> {{Assertions.assertEquals(double, double)}}|
> |{{assertEquals(String, double[], double[], double)}}|Equivalent to
> {{Assertions.assertArrayEquals(double[], double[], double, String)}}|
> |{{assertEquals(String, float[], float[], float)}}|Equivalent to
> {{Assertions.assertArrayEquals(float[], float[], float, String)}}|
> |{{equalsIncludingNaN(double, double, double)}}|Equivalent to
> {{Precision.equalsIncludingNaN(double, double, double)}}|
> Finally, the following methods from the class {{TestUtils}} in the module
> _commons-numbers-complex_ are redundant:
> |{{assertEquals(double, double, double)}}|Equivalent to
> {{Assertions.assertEquals(double, double, double)}}, as explained above|
> |{{assertEquals(String, double, double, double)}}|Equivalent to
> {{Assertions.assertEquals(double, double, double, String)}}, as explained
> above|
> |{{assertSame(double, double)}}|Equivalent to
> {{Assertions.assertEquals(double, double)}}|
> |{{assertEquals(double[], double[], double)}}|Equivalent to
> {{Assertions.assertArrayEquals(double[], double[], double)}}|
> |{{assertEquals(String, double[], double[], double)}}|Equivalent to
> {{Assertions.assertArrayEquals(double[], double[], double, String)}}|
> |{{assertEquals(String, float[], float[], float)}}|Equivalent to
> {{Assertions.assertArrayEquals(float[], float[], float, String)}}|
> |{{equalsIncludingNaN(double, double, double)}}|Equivalent to
> {{Precision.equalsIncludingNaN(double, double, double)}}|
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Reopened] (NUMBERS-117) Redundant methods in several TestUtils classes
[
https://issues.apache.org/jira/browse/NUMBERS-117?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Heinrich Bohne reopened NUMBERS-117:
> Redundant methods in several TestUtils classes
> --
>
> Key: NUMBERS-117
> URL: https://issues.apache.org/jira/browse/NUMBERS-117
> Project: Commons Numbers
> Issue Type: Improvement
>Reporter: Heinrich Bohne
>Priority: Minor
> Time Spent: 20m
> Remaining Estimate: 0h
>
> The following methods from the class {{TestUtils}} in the module
> _commons-numbers-core_ are redundant:
> |{{assertEquals(double, double, double)}}|{{Assertions.assertEquals(double,
> double, double)}} already considers two {{NaN}} values equal, so the two
> methods are equivalent.|
> |{{assertEquals(String, double, double, double)}}|Equivalent to
> {{Assertions.assertEquals(double, double, double, String)}}, as explained
> above|
> |{{assertSame(double, double)}}|Equivalent to
> {{Assertions.assertEquals(double, double)}}|
> |{{assertEquals(double[], double[], double)}}|Equivalent to
> {{Assertions.assertArrayEquals(double[], double[], double)}}|
> |{{assertEquals(String, double[], double[], double)}}|Equivalent to
> {{Assertions.assertArrayEquals(double[], double[], double, String)}}|
> |{{assertEquals(String, float[], float[], float)}}|Equivalent to
> {{Assertions.assertArrayEquals(float[], float[], float, String)}}|
> Similarly, the following methods from the class {{TestUtils}} in the module
> _commons-numbers-complex-streams_ are redundant:
> |{{assertEquals(double, double, double)}}|Equivalent to
> {{Assertions.assertEquals(double, double, double)}}, as explained above|
> |{{assertEquals(String, double, double, double)}}|Equivalent to
> {{Assertions.assertEquals(double, double, double, String)}}, as explained
> above|
> |{{assertSame(double[], double[])}}|Equivalent to
> {{Assertions.assertArrayEquals(double[], double[])}}|
> |{{assertSame(float[], float[])}}|Equivalent to
> {{Assertions.assertArrayEquals(float[], float[])}}|
> |{{assertSame(double, double)}}|Equivalent to
> {{Assertions.assertEquals(double, double)}}|
> |{{assertEquals(String, double[], double[], double)}}|Equivalent to
> {{Assertions.assertArrayEquals(double[], double[], double, String)}}|
> |{{assertEquals(String, float[], float[], float)}}|Equivalent to
> {{Assertions.assertArrayEquals(float[], float[], float, String)}}|
> |{{equalsIncludingNaN(double, double, double)}}|Equivalent to
> {{Precision.equalsIncludingNaN(double, double, double)}}|
> Finally, the following methods from the class {{TestUtils}} in the module
> _commons-numbers-complex_ are redundant:
> |{{assertEquals(double, double, double)}}|Equivalent to
> {{Assertions.assertEquals(double, double, double)}}, as explained above|
> |{{assertEquals(String, double, double, double)}}|Equivalent to
> {{Assertions.assertEquals(double, double, double, String)}}, as explained
> above|
> |{{assertSame(double, double)}}|Equivalent to
> {{Assertions.assertEquals(double, double)}}|
> |{{assertEquals(double[], double[], double)}}|Equivalent to
> {{Assertions.assertArrayEquals(double[], double[], double)}}|
> |{{assertEquals(String, double[], double[], double)}}|Equivalent to
> {{Assertions.assertArrayEquals(double[], double[], double, String)}}|
> |{{assertEquals(String, float[], float[], float)}}|Equivalent to
> {{Assertions.assertArrayEquals(float[], float[], float, String)}}|
> |{{equalsIncludingNaN(double, double, double)}}|Equivalent to
> {{Precision.equalsIncludingNaN(double, double, double)}}|
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Updated] (NUMBERS-115) Migrate to JUnit 5
[ https://issues.apache.org/jira/browse/NUMBERS-115?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Heinrich Bohne updated NUMBERS-115: --- Fix Version/s: 1.0 > Migrate to JUnit 5 > -- > > Key: NUMBERS-115 > URL: https://issues.apache.org/jira/browse/NUMBERS-115 > Project: Commons Numbers > Issue Type: Task >Reporter: Heinrich Bohne >Priority: Minor > Fix For: 1.0 > > Time Spent: 20m > Remaining Estimate: 0h > > As has been > [discussed|http://mail-archives.apache.org/mod_mbox/commons-dev/201905.mbox/%3C8883d6fc-2a22-608a-af3d-96272cd425c9%40gmx.at%3E] > on the developers' mailing list, the whole project can be migrated to JUnit > 5. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Resolved] (NUMBERS-114) Class LinearCombinationTest in arrays module does not compile
[
https://issues.apache.org/jira/browse/NUMBERS-114?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Heinrich Bohne resolved NUMBERS-114.
Resolution: Fixed
Fix Version/s: 1.0
> Class LinearCombinationTest in arrays module does not compile
> -
>
> Key: NUMBERS-114
> URL: https://issues.apache.org/jira/browse/NUMBERS-114
> Project: Commons Numbers
> Issue Type: Bug
>Reporter: Heinrich Bohne
>Priority: Major
> Fix For: 1.0
>
>
> The recent merge of the fraction-dev branch into the master branch broke the
> class {{LinearCombinationTest}} in the arrays module. This class accesses a
> constructor {{BigFraction(long, long)}}, which was available before the merge
> but is no longer so.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Reopened] (NUMBERS-115) Migrate to JUnit 5
[ https://issues.apache.org/jira/browse/NUMBERS-115?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Heinrich Bohne reopened NUMBERS-115: > Migrate to JUnit 5 > -- > > Key: NUMBERS-115 > URL: https://issues.apache.org/jira/browse/NUMBERS-115 > Project: Commons Numbers > Issue Type: Task >Reporter: Heinrich Bohne >Priority: Minor > Time Spent: 20m > Remaining Estimate: 0h > > As has been > [discussed|http://mail-archives.apache.org/mod_mbox/commons-dev/201905.mbox/%3C8883d6fc-2a22-608a-af3d-96272cd425c9%40gmx.at%3E] > on the developers' mailing list, the whole project can be migrated to JUnit > 5. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Resolved] (NUMBERS-115) Migrate to JUnit 5
[ https://issues.apache.org/jira/browse/NUMBERS-115?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Heinrich Bohne resolved NUMBERS-115. Resolution: Fixed > Migrate to JUnit 5 > -- > > Key: NUMBERS-115 > URL: https://issues.apache.org/jira/browse/NUMBERS-115 > Project: Commons Numbers > Issue Type: Task >Reporter: Heinrich Bohne >Priority: Minor > Time Spent: 20m > Remaining Estimate: 0h > > As has been > [discussed|http://mail-archives.apache.org/mod_mbox/commons-dev/201905.mbox/%3C8883d6fc-2a22-608a-af3d-96272cd425c9%40gmx.at%3E] > on the developers' mailing list, the whole project can be migrated to JUnit > 5. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Reopened] (NUMBERS-114) Class LinearCombinationTest in arrays module does not compile
[
https://issues.apache.org/jira/browse/NUMBERS-114?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Heinrich Bohne reopened NUMBERS-114:
> Class LinearCombinationTest in arrays module does not compile
> -
>
> Key: NUMBERS-114
> URL: https://issues.apache.org/jira/browse/NUMBERS-114
> Project: Commons Numbers
> Issue Type: Bug
>Reporter: Heinrich Bohne
>Priority: Major
>
> The recent merge of the fraction-dev branch into the master branch broke the
> class {{LinearCombinationTest}} in the arrays module. This class accesses a
> constructor {{BigFraction(long, long)}}, which was available before the merge
> but is no longer so.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Resolved] (NUMBERS-113) Broken/deprecated/old JUnit dependencies in pom.xml
[
https://issues.apache.org/jira/browse/NUMBERS-113?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Heinrich Bohne resolved NUMBERS-113.
Resolution: Fixed
Fix Version/s: 1.0
> Broken/deprecated/old JUnit dependencies in pom.xml
> ---
>
> Key: NUMBERS-113
> URL: https://issues.apache.org/jira/browse/NUMBERS-113
> Project: Commons Numbers
> Issue Type: Bug
>Reporter: Heinrich Bohne
>Priority: Major
> Fix For: 1.0
>
> Time Spent: 1h 20m
> Remaining Estimate: 0h
>
> The recent
> [commit|https://github.com/apache/commons-numbers/commit/35b53cff1a9fc8c9a5b4a44bd33d22c959c4]
> where JUnit 5 was introduced seems to have broken something. Apparently, the
> version number for the junit-platform-runner dependency (version 5.0.0-M4)
> does not exist.
> Also, the latest stable version of junit-jupiter-engine is 5.4.2, but the
> pom.xml declares a version number 5.0.0-M4 also for this dependency.
> Besides, I'm getting a warning when trying to run tests:
> bq. The junit-platform-surefire-provider has been deprecated and is scheduled
> to be removed in JUnit Platform 1.4. Please use the built-in support in Maven
> Surefire >= 2.22.0 instead.
> I'm not sure what this means, but I assume it has something to do with the
> dependencies configured for the surefire plugin, which were not there before.
> Besides, I think the appropriate scope for the junit-jupiter-engine
> dependency is {{test}} (currently, it is not set, so it defaults to
> {{compile}}).
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Reopened] (NUMBERS-104) Speed up trial division
[
https://issues.apache.org/jira/browse/NUMBERS-104?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Heinrich Bohne reopened NUMBERS-104:
> Speed up trial division
> ---
>
> Key: NUMBERS-104
> URL: https://issues.apache.org/jira/browse/NUMBERS-104
> Project: Commons Numbers
> Issue Type: Improvement
> Components: primes
>Reporter: Heinrich Bohne
>Priority: Minor
> Time Spent: 50m
> Remaining Estimate: 0h
>
> Currently, the method {{SmallPrimes.boundedTrialDivision(int, int,
> List)}} skips multiples of 2 and 3, thereby reducing the amount of
> integers to be tried to 1/3 of all integers. This can be improved at the cost
> of extra memory by extending the set of prime factors to be skipped in the
> trial division, for example, to 2, 3, 5, 7, and 11.
> However, instead of code duplication, which is the way the code currently
> achieves this, a way to implement this could be:
> * First, precompute and store all integers between 0 (inclusive) and
> 2⋅3⋅5⋅7⋅11=2310 (exclusive) that are not divisible by any of those 5 prime
> numbers (480 numbers in total, according to my experiments).
> * Then, when performing the trial division, one only needs to try out numbers
> of the form 2310⋅k + c, where k is a non-negative integer and c is one of the
> 480 numbers described in the previous step.
> That way, the amount of integers to be tried will be 480/2310 ≈ 20.78%,
> instead of 1/3 ≈ 33.33%, which is a speedup of about 60.42% compared to the
> current algorithm. Of course, with more prime factors eliminated, less
> numbers will have to be tried, but it seems that the returns are quickly
> diminishing compared to the required memory. For example, when also
> eliminating the prime factors 13, 17 and 19, the memory required increases to
> 1658880 integers, whereas the percentage of integers to be tried only drops
> to about 17.10%.
> Since the class {{SmallPrimes}} already stores an array containing the first
> 512 prime numbers, a 480-element {{int}} array seems like a reasonable size
> and a small price to pay for a 60.42% speedup.
> I'm just not entirely sure whether this should go here or on the developer
> mailing list. Since this is actually not a new idea, but just an enhancement
> of an already existing idea, I'm putting it here, but on the other hand, it
> requires some re-writing and some new code, so maybe I'm wrong.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Resolved] (NUMBERS-104) Speed up trial division
[
https://issues.apache.org/jira/browse/NUMBERS-104?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Heinrich Bohne resolved NUMBERS-104.
Resolution: Fixed
Fix Version/s: 1.0
> Speed up trial division
> ---
>
> Key: NUMBERS-104
> URL: https://issues.apache.org/jira/browse/NUMBERS-104
> Project: Commons Numbers
> Issue Type: Improvement
> Components: primes
>Reporter: Heinrich Bohne
>Priority: Minor
> Fix For: 1.0
>
> Time Spent: 50m
> Remaining Estimate: 0h
>
> Currently, the method {{SmallPrimes.boundedTrialDivision(int, int,
> List)}} skips multiples of 2 and 3, thereby reducing the amount of
> integers to be tried to 1/3 of all integers. This can be improved at the cost
> of extra memory by extending the set of prime factors to be skipped in the
> trial division, for example, to 2, 3, 5, 7, and 11.
> However, instead of code duplication, which is the way the code currently
> achieves this, a way to implement this could be:
> * First, precompute and store all integers between 0 (inclusive) and
> 2⋅3⋅5⋅7⋅11=2310 (exclusive) that are not divisible by any of those 5 prime
> numbers (480 numbers in total, according to my experiments).
> * Then, when performing the trial division, one only needs to try out numbers
> of the form 2310⋅k + c, where k is a non-negative integer and c is one of the
> 480 numbers described in the previous step.
> That way, the amount of integers to be tried will be 480/2310 ≈ 20.78%,
> instead of 1/3 ≈ 33.33%, which is a speedup of about 60.42% compared to the
> current algorithm. Of course, with more prime factors eliminated, less
> numbers will have to be tried, but it seems that the returns are quickly
> diminishing compared to the required memory. For example, when also
> eliminating the prime factors 13, 17 and 19, the memory required increases to
> 1658880 integers, whereas the percentage of integers to be tried only drops
> to about 17.10%.
> Since the class {{SmallPrimes}} already stores an array containing the first
> 512 prime numbers, a 480-element {{int}} array seems like a reasonable size
> and a small price to pay for a 60.42% speedup.
> I'm just not entirely sure whether this should go here or on the developer
> mailing list. Since this is actually not a new idea, but just an enhancement
> of an already existing idea, I'm putting it here, but on the other hand, it
> requires some re-writing and some new code, so maybe I'm wrong.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Updated] (NUMBERS-100) Code in file FractionTest.java is unsatisfactory
[
https://issues.apache.org/jira/browse/NUMBERS-100?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Heinrich Bohne updated NUMBERS-100:
---
Fix Version/s: 1.0
> Code in file FractionTest.java is unsatisfactory
>
>
> Key: NUMBERS-100
> URL: https://issues.apache.org/jira/browse/NUMBERS-100
> Project: Commons Numbers
> Issue Type: Improvement
> Components: fraction
>Reporter: Heinrich Bohne
>Priority: Trivial
> Fix For: 1.0
>
> Time Spent: 1h 50m
> Remaining Estimate: 0h
>
> The following characteristics of the file {{FractionTest.java}} can be
> improved:
> * The second-to-last try-catch-block in the method {{testAdd()}} is a
> duplicate of the preceding try-catch-block and is therefore redundant.
> * In the method {{testPow()}}, the conditions {{assertFraction(9, 49,
> a.pow(2))}} and {{assertFraction(49, 9, a.pow(-2))}} are tested twice each
> (once in the block after {{a}}'s declaration, and a second time in the block
> after {{b}}'s declaration. This is probably a typo.
> * The last two assertions in the method {{testGetReducedFraction()}} pass the
> parameters to the method {{Assert.assertEquals(long, long)}} in the wrong
> order (the expected value should go first).
> * Several methods in this class contain a number of tests that use shared
> local variables but are completely independent of each other because these
> local variables get assigned new values at the beginning of a test. The fact
> that the scope of these local variables encompasses all those independent
> tests makes the code look more confusing than necessary.
> * Except for the method {{testGoldenRatio()}}, the throwing of an exception
> is tested with a construct involving the swallowing of an exception, rather
> than an explicit syntax.
> * The helper method {{assertFraction(int, int, Fraction)}} is neglected
> throughout large sections of the class in favor of
> {{Assert.assertEquals(long, long)}} pairs, increasing the amount of code
> duplication.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (MATH-1492) Replace usages of commons-numbers-core methods with equivalent methods from java.lang.Math
[
https://issues.apache.org/jira/browse/MATH-1492?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16868517#comment-16868517
]
Heinrich Bohne commented on MATH-1492:
--
Whoops, OK, thanks for the heads-up. The description of the function led me to
think that closing a ticket has to be done by the person who reported it.
> Replace usages of commons-numbers-core methods with equivalent methods from
> java.lang.Math
> --
>
> Key: MATH-1492
> URL: https://issues.apache.org/jira/browse/MATH-1492
> Project: Commons Math
> Issue Type: Task
>Reporter: Heinrich Bohne
>Priority: Minor
> Fix For: 4.0
>
> Time Spent: 20m
> Remaining Estimate: 0h
>
> The usages of the following methods from
> {{org.apache.commons.numbers.core.ArithmeticUtils}}:
> {{addAndCheck(int, int)}}
> {{addAndCheck(long, long)}}
> {{mulAndCheck(int, int)}}
> {{mulAndCheck(long, long)}}
> {{subAndCheck(int, int)}}
> {{subAndCheck(long, long)}}
> Can be replaced with the following equivalent methods from {{java.lang.Math}}:
> {{addExact(int, int)}}
> {{addExact(long, long)}}
> {{multiplyExact(int, int)}}
> {{multiplyExact(long, long)}}
> {{subtractExact(int, int)}}
> {{subtractExact(long, long)}}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Resolved] (NUMBERS-100) Code in file FractionTest.java is unsatisfactory
[
https://issues.apache.org/jira/browse/NUMBERS-100?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Heinrich Bohne resolved NUMBERS-100.
Resolution: Fixed
> Code in file FractionTest.java is unsatisfactory
>
>
> Key: NUMBERS-100
> URL: https://issues.apache.org/jira/browse/NUMBERS-100
> Project: Commons Numbers
> Issue Type: Improvement
> Components: fraction
>Reporter: Heinrich Bohne
>Priority: Trivial
> Time Spent: 1h 50m
> Remaining Estimate: 0h
>
> The following characteristics of the file {{FractionTest.java}} can be
> improved:
> * The second-to-last try-catch-block in the method {{testAdd()}} is a
> duplicate of the preceding try-catch-block and is therefore redundant.
> * In the method {{testPow()}}, the conditions {{assertFraction(9, 49,
> a.pow(2))}} and {{assertFraction(49, 9, a.pow(-2))}} are tested twice each
> (once in the block after {{a}}'s declaration, and a second time in the block
> after {{b}}'s declaration. This is probably a typo.
> * The last two assertions in the method {{testGetReducedFraction()}} pass the
> parameters to the method {{Assert.assertEquals(long, long)}} in the wrong
> order (the expected value should go first).
> * Several methods in this class contain a number of tests that use shared
> local variables but are completely independent of each other because these
> local variables get assigned new values at the beginning of a test. The fact
> that the scope of these local variables encompasses all those independent
> tests makes the code look more confusing than necessary.
> * Except for the method {{testGoldenRatio()}}, the throwing of an exception
> is tested with a construct involving the swallowing of an exception, rather
> than an explicit syntax.
> * The helper method {{assertFraction(int, int, Fraction)}} is neglected
> throughout large sections of the class in favor of
> {{Assert.assertEquals(long, long)}} pairs, increasing the amount of code
> duplication.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Reopened] (NUMBERS-100) Code in file FractionTest.java is unsatisfactory
[
https://issues.apache.org/jira/browse/NUMBERS-100?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Heinrich Bohne reopened NUMBERS-100:
> Code in file FractionTest.java is unsatisfactory
>
>
> Key: NUMBERS-100
> URL: https://issues.apache.org/jira/browse/NUMBERS-100
> Project: Commons Numbers
> Issue Type: Improvement
> Components: fraction
>Reporter: Heinrich Bohne
>Priority: Trivial
> Time Spent: 1h 50m
> Remaining Estimate: 0h
>
> The following characteristics of the file {{FractionTest.java}} can be
> improved:
> * The second-to-last try-catch-block in the method {{testAdd()}} is a
> duplicate of the preceding try-catch-block and is therefore redundant.
> * In the method {{testPow()}}, the conditions {{assertFraction(9, 49,
> a.pow(2))}} and {{assertFraction(49, 9, a.pow(-2))}} are tested twice each
> (once in the block after {{a}}'s declaration, and a second time in the block
> after {{b}}'s declaration. This is probably a typo.
> * The last two assertions in the method {{testGetReducedFraction()}} pass the
> parameters to the method {{Assert.assertEquals(long, long)}} in the wrong
> order (the expected value should go first).
> * Several methods in this class contain a number of tests that use shared
> local variables but are completely independent of each other because these
> local variables get assigned new values at the beginning of a test. The fact
> that the scope of these local variables encompasses all those independent
> tests makes the code look more confusing than necessary.
> * Except for the method {{testGoldenRatio()}}, the throwing of an exception
> is tested with a construct involving the swallowing of an exception, rather
> than an explicit syntax.
> * The helper method {{assertFraction(int, int, Fraction)}} is neglected
> throughout large sections of the class in favor of
> {{Assert.assertEquals(long, long)}} pairs, increasing the amount of code
> duplication.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Resolved] (MATH-1492) Replace usages of commons-numbers-core methods with equivalent methods from java.lang.Math
[
https://issues.apache.org/jira/browse/MATH-1492?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Heinrich Bohne resolved MATH-1492.
--
Resolution: Resolved
> Replace usages of commons-numbers-core methods with equivalent methods from
> java.lang.Math
> --
>
> Key: MATH-1492
> URL: https://issues.apache.org/jira/browse/MATH-1492
> Project: Commons Math
> Issue Type: Task
>Reporter: Heinrich Bohne
>Priority: Minor
> Fix For: 4.0
>
> Time Spent: 20m
> Remaining Estimate: 0h
>
> The usages of the following methods from
> {{org.apache.commons.numbers.core.ArithmeticUtils}}:
> {{addAndCheck(int, int)}}
> {{addAndCheck(long, long)}}
> {{mulAndCheck(int, int)}}
> {{mulAndCheck(long, long)}}
> {{subAndCheck(int, int)}}
> {{subAndCheck(long, long)}}
> Can be replaced with the following equivalent methods from {{java.lang.Math}}:
> {{addExact(int, int)}}
> {{addExact(long, long)}}
> {{multiplyExact(int, int)}}
> {{multiplyExact(long, long)}}
> {{subtractExact(int, int)}}
> {{subtractExact(long, long)}}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Reopened] (MATH-1492) Replace usages of commons-numbers-core methods with equivalent methods from java.lang.Math
[
https://issues.apache.org/jira/browse/MATH-1492?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Heinrich Bohne reopened MATH-1492:
--
> Replace usages of commons-numbers-core methods with equivalent methods from
> java.lang.Math
> --
>
> Key: MATH-1492
> URL: https://issues.apache.org/jira/browse/MATH-1492
> Project: Commons Math
> Issue Type: Task
>Reporter: Heinrich Bohne
>Priority: Minor
> Fix For: 4.0
>
> Time Spent: 20m
> Remaining Estimate: 0h
>
> The usages of the following methods from
> {{org.apache.commons.numbers.core.ArithmeticUtils}}:
> {{addAndCheck(int, int)}}
> {{addAndCheck(long, long)}}
> {{mulAndCheck(int, int)}}
> {{mulAndCheck(long, long)}}
> {{subAndCheck(int, int)}}
> {{subAndCheck(long, long)}}
> Can be replaced with the following equivalent methods from {{java.lang.Math}}:
> {{addExact(int, int)}}
> {{addExact(long, long)}}
> {{multiplyExact(int, int)}}
> {{multiplyExact(long, long)}}
> {{subtractExact(int, int)}}
> {{subtractExact(long, long)}}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Closed] (MATH-1492) Replace usages of commons-numbers-core methods with equivalent methods from java.lang.Math
[
https://issues.apache.org/jira/browse/MATH-1492?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Heinrich Bohne closed MATH-1492.
> Replace usages of commons-numbers-core methods with equivalent methods from
> java.lang.Math
> --
>
> Key: MATH-1492
> URL: https://issues.apache.org/jira/browse/MATH-1492
> Project: Commons Math
> Issue Type: Task
>Reporter: Heinrich Bohne
>Priority: Minor
> Fix For: 4.0
>
> Time Spent: 20m
> Remaining Estimate: 0h
>
> The usages of the following methods from
> {{org.apache.commons.numbers.core.ArithmeticUtils}}:
> {{addAndCheck(int, int)}}
> {{addAndCheck(long, long)}}
> {{mulAndCheck(int, int)}}
> {{mulAndCheck(long, long)}}
> {{subAndCheck(int, int)}}
> {{subAndCheck(long, long)}}
> Can be replaced with the following equivalent methods from {{java.lang.Math}}:
> {{addExact(int, int)}}
> {{addExact(long, long)}}
> {{multiplyExact(int, int)}}
> {{multiplyExact(long, long)}}
> {{subtractExact(int, int)}}
> {{subtractExact(long, long)}}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Created] (MATH-1492) Replace usages of commons-numbers-core methods with equivalent methods from java.lang.Math
Heinrich Bohne created MATH-1492:
Summary: Replace usages of commons-numbers-core methods with
equivalent methods from java.lang.Math
Key: MATH-1492
URL: https://issues.apache.org/jira/browse/MATH-1492
Project: Commons Math
Issue Type: Task
Reporter: Heinrich Bohne
Fix For: 4.0
The usages of the following methods from
{{org.apache.commons.numbers.core.ArithmeticUtils}}:
{{addAndCheck(int, int)}}
{{addAndCheck(long, long)}}
{{mulAndCheck(int, int)}}
{{mulAndCheck(long, long)}}
{{subAndCheck(int, int)}}
{{subAndCheck(long, long)}}
Can be replaced with the following equivalent methods from {{java.lang.Math}}:
{{addExact(int, int)}}
{{addExact(long, long)}}
{{multiplyExact(int, int)}}
{{multiplyExact(long, long)}}
{{subtractExact(int, int)}}
{{subtractExact(long, long)}}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Closed] (NUMBERS-100) Code in file FractionTest.java is unsatisfactory
[
https://issues.apache.org/jira/browse/NUMBERS-100?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Heinrich Bohne closed NUMBERS-100.
--
Resolution: Fixed
> Code in file FractionTest.java is unsatisfactory
>
>
> Key: NUMBERS-100
> URL: https://issues.apache.org/jira/browse/NUMBERS-100
> Project: Commons Numbers
> Issue Type: Improvement
> Components: fraction
>Reporter: Heinrich Bohne
>Priority: Trivial
> Time Spent: 1h 50m
> Remaining Estimate: 0h
>
> The following characteristics of the file {{FractionTest.java}} can be
> improved:
> * The second-to-last try-catch-block in the method {{testAdd()}} is a
> duplicate of the preceding try-catch-block and is therefore redundant.
> * In the method {{testPow()}}, the conditions {{assertFraction(9, 49,
> a.pow(2))}} and {{assertFraction(49, 9, a.pow(-2))}} are tested twice each
> (once in the block after {{a}}'s declaration, and a second time in the block
> after {{b}}'s declaration. This is probably a typo.
> * The last two assertions in the method {{testGetReducedFraction()}} pass the
> parameters to the method {{Assert.assertEquals(long, long)}} in the wrong
> order (the expected value should go first).
> * Several methods in this class contain a number of tests that use shared
> local variables but are completely independent of each other because these
> local variables get assigned new values at the beginning of a test. The fact
> that the scope of these local variables encompasses all those independent
> tests makes the code look more confusing than necessary.
> * Except for the method {{testGoldenRatio()}}, the throwing of an exception
> is tested with a construct involving the swallowing of an exception, rather
> than an explicit syntax.
> * The helper method {{assertFraction(int, int, Fraction)}} is neglected
> throughout large sections of the class in favor of
> {{Assert.assertEquals(long, long)}} pairs, increasing the amount of code
> duplication.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Closed] (NUMBERS-117) Redundant methods in several TestUtils classes
[
https://issues.apache.org/jira/browse/NUMBERS-117?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Heinrich Bohne closed NUMBERS-117.
--
Resolution: Fixed
> Redundant methods in several TestUtils classes
> --
>
> Key: NUMBERS-117
> URL: https://issues.apache.org/jira/browse/NUMBERS-117
> Project: Commons Numbers
> Issue Type: Improvement
>Reporter: Heinrich Bohne
>Priority: Minor
> Time Spent: 20m
> Remaining Estimate: 0h
>
> The following methods from the class {{TestUtils}} in the module
> _commons-numbers-core_ are redundant:
> |{{assertEquals(double, double, double)}}|{{Assertions.assertEquals(double,
> double, double)}} already considers two {{NaN}} values equal, so the two
> methods are equivalent.|
> |{{assertEquals(String, double, double, double)}}|Equivalent to
> {{Assertions.assertEquals(double, double, double, String)}}, as explained
> above|
> |{{assertSame(double, double)}}|Equivalent to
> {{Assertions.assertEquals(double, double)}}|
> |{{assertEquals(double[], double[], double)}}|Equivalent to
> {{Assertions.assertArrayEquals(double[], double[], double)}}|
> |{{assertEquals(String, double[], double[], double)}}|Equivalent to
> {{Assertions.assertArrayEquals(double[], double[], double, String)}}|
> |{{assertEquals(String, float[], float[], float)}}|Equivalent to
> {{Assertions.assertArrayEquals(float[], float[], float, String)}}|
> Similarly, the following methods from the class {{TestUtils}} in the module
> _commons-numbers-complex-streams_ are redundant:
> |{{assertEquals(double, double, double)}}|Equivalent to
> {{Assertions.assertEquals(double, double, double)}}, as explained above|
> |{{assertEquals(String, double, double, double)}}|Equivalent to
> {{Assertions.assertEquals(double, double, double, String)}}, as explained
> above|
> |{{assertSame(double[], double[])}}|Equivalent to
> {{Assertions.assertArrayEquals(double[], double[])}}|
> |{{assertSame(float[], float[])}}|Equivalent to
> {{Assertions.assertArrayEquals(float[], float[])}}|
> |{{assertSame(double, double)}}|Equivalent to
> {{Assertions.assertEquals(double, double)}}|
> |{{assertEquals(String, double[], double[], double)}}|Equivalent to
> {{Assertions.assertArrayEquals(double[], double[], double, String)}}|
> |{{assertEquals(String, float[], float[], float)}}|Equivalent to
> {{Assertions.assertArrayEquals(float[], float[], float, String)}}|
> |{{equalsIncludingNaN(double, double, double)}}|Equivalent to
> {{Precision.equalsIncludingNaN(double, double, double)}}|
> Finally, the following methods from the class {{TestUtils}} in the module
> _commons-numbers-complex_ are redundant:
> |{{assertEquals(double, double, double)}}|Equivalent to
> {{Assertions.assertEquals(double, double, double)}}, as explained above|
> |{{assertEquals(String, double, double, double)}}|Equivalent to
> {{Assertions.assertEquals(double, double, double, String)}}, as explained
> above|
> |{{assertSame(double, double)}}|Equivalent to
> {{Assertions.assertEquals(double, double)}}|
> |{{assertEquals(double[], double[], double)}}|Equivalent to
> {{Assertions.assertArrayEquals(double[], double[], double)}}|
> |{{assertEquals(String, double[], double[], double)}}|Equivalent to
> {{Assertions.assertArrayEquals(double[], double[], double, String)}}|
> |{{assertEquals(String, float[], float[], float)}}|Equivalent to
> {{Assertions.assertArrayEquals(float[], float[], float, String)}}|
> |{{equalsIncludingNaN(double, double, double)}}|Equivalent to
> {{Precision.equalsIncludingNaN(double, double, double)}}|
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Created] (NUMBERS-117) Redundant methods in several TestUtils classes
Heinrich Bohne created NUMBERS-117:
--
Summary: Redundant methods in several TestUtils classes
Key: NUMBERS-117
URL: https://issues.apache.org/jira/browse/NUMBERS-117
Project: Commons Numbers
Issue Type: Improvement
Reporter: Heinrich Bohne
The following methods from the class {{TestUtils}} in the module
_commons-numbers-core_ are redundant:
|{{assertEquals(double, double, double)}}|{{Assertions.assertEquals(double,
double, double)}} already considers two {{NaN}} values equal, so the two
methods are equivalent.|
|{{assertEquals(String, double, double, double)}}|Equivalent to
{{Assertions.assertEquals(double, double, double, String)}}, as explained above|
|{{assertSame(double, double)}}|Equivalent to {{Assertions.assertEquals(double,
double)}}|
|{{assertEquals(double[], double[], double)}}|Equivalent to
{{Assertions.assertArrayEquals(double[], double[], double)}}|
|{{assertEquals(String, double[], double[], double)}}|Equivalent to
{{Assertions.assertArrayEquals(double[], double[], double, String)}}|
|{{assertEquals(String, float[], float[], float)}}|Equivalent to
{{Assertions.assertArrayEquals(float[], float[], float, String)}}|
Similarly, the following methods from the class {{TestUtils}} in the module
_commons-numbers-complex-streams_ are redundant:
|{{assertEquals(double, double, double)}}|Equivalent to
{{Assertions.assertEquals(double, double, double)}}, as explained above|
|{{assertEquals(String, double, double, double)}}|Equivalent to
{{Assertions.assertEquals(double, double, double, String)}}, as explained above|
|{{assertSame(double[], double[])}}|Equivalent to
{{Assertions.assertArrayEquals(double[], double[])}}|
|{{assertSame(float[], float[])}}|Equivalent to
{{Assertions.assertArrayEquals(float[], float[])}}|
|{{assertSame(double, double)}}|Equivalent to {{Assertions.assertEquals(double,
double)}}|
|{{assertEquals(String, double[], double[], double)}}|Equivalent to
{{Assertions.assertArrayEquals(double[], double[], double, String)}}|
|{{assertEquals(String, float[], float[], float)}}|Equivalent to
{{Assertions.assertArrayEquals(float[], float[], float, String)}}|
|{{equalsIncludingNaN(double, double, double)}}|Equivalent to
{{Precision.equalsIncludingNaN(double, double, double)}}|
Finally, the following methods from the class {{TestUtils}} in the module
_commons-numbers-complex_ are redundant:
|{{assertEquals(double, double, double)}}|Equivalent to
{{Assertions.assertEquals(double, double, double)}}, as explained above|
|{{assertEquals(String, double, double, double)}}|Equivalent to
{{Assertions.assertEquals(double, double, double, String)}}, as explained above|
|{{assertSame(double, double)}}|Equivalent to {{Assertions.assertEquals(double,
double)}}|
|{{assertEquals(double[], double[], double)}}|Equivalent to
{{Assertions.assertArrayEquals(double[], double[], double)}}|
|{{assertEquals(String, double[], double[], double)}}|Equivalent to
{{Assertions.assertArrayEquals(double[], double[], double, String)}}|
|{{assertEquals(String, float[], float[], float)}}|Equivalent to
{{Assertions.assertArrayEquals(float[], float[], float, String)}}|
|{{equalsIncludingNaN(double, double, double)}}|Equivalent to
{{Precision.equalsIncludingNaN(double, double, double)}}|
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Created] (NUMBERS-116) Remove redundant methods in ArithmeticUtils
Heinrich Bohne created NUMBERS-116:
--
Summary: Remove redundant methods in ArithmeticUtils
Key: NUMBERS-116
URL: https://issues.apache.org/jira/browse/NUMBERS-116
Project: Commons Numbers
Issue Type: Improvement
Components: core
Reporter: Heinrich Bohne
As has been
[discussed|http://mail-archives.apache.org/mod_mbox/commons-dev/201906.mbox/%3C940f9ff0-0b25-cd31-ddb3-a95ca777ba06%40gmx.at%3E]
on the developers' mailing list, the following methods from the class
{{ArithmeticUtils}} can be removed:
{{addAndCheck(int, int)}}
{{addAndCheck(long, long)}}
{{mulAndCheck(int, int)}}
{{mulAndCheck(long, long)}}
{{subAndCheck(int, int)}}
{{subAndCheck(long, long)}}
And their usages replaced with the following equivalent methods from
{{java.lang.Math}}:
{{addExact(int, int)}}
{{addExact(long, long)}}
{{multiplyExact(int, int)}}
{{multiplyExact(long, long)}}
{{subtractExact(int, int)}}
{{subtractExact(long, long)}}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Closed] (NUMBERS-115) Migrate to JUnit 5
[ https://issues.apache.org/jira/browse/NUMBERS-115?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Heinrich Bohne closed NUMBERS-115. -- Resolution: Fixed > Migrate to JUnit 5 > -- > > Key: NUMBERS-115 > URL: https://issues.apache.org/jira/browse/NUMBERS-115 > Project: Commons Numbers > Issue Type: Task >Reporter: Heinrich Bohne >Priority: Minor > Time Spent: 20m > Remaining Estimate: 0h > > As has been > [discussed|http://mail-archives.apache.org/mod_mbox/commons-dev/201905.mbox/%3C8883d6fc-2a22-608a-af3d-96272cd425c9%40gmx.at%3E] > on the developers' mailing list, the whole project can be migrated to JUnit > 5. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (NUMBERS-115) Migrate to JUnit 5
[ https://issues.apache.org/jira/browse/NUMBERS-115?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16866610#comment-16866610 ] Heinrich Bohne commented on NUMBERS-115: Indeed, the dependencies still need to be taken care of. Grimreaper said that he would do it after the merge, so I assumed he had a reason not to do it in [PR #48|https://github.com/apache/commons-numbers/pull/48], otherwise I would have already included it in the pull request. Should I also relate the new pull request to this JIRA ticket? > Migrate to JUnit 5 > -- > > Key: NUMBERS-115 > URL: https://issues.apache.org/jira/browse/NUMBERS-115 > Project: Commons Numbers > Issue Type: Task >Reporter: Heinrich Bohne >Priority: Minor > > As has been > [discussed|http://mail-archives.apache.org/mod_mbox/commons-dev/201905.mbox/%3C8883d6fc-2a22-608a-af3d-96272cd425c9%40gmx.at%3E] > on the developers' mailing list, the whole project can be migrated to JUnit > 5. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Updated] (NUMBERS-115) Migrate to JUnit 5
[ https://issues.apache.org/jira/browse/NUMBERS-115?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Heinrich Bohne updated NUMBERS-115: --- External issue URL: (was: https://github.com/apache/commons-numbers/pull/48) > Migrate to JUnit 5 > -- > > Key: NUMBERS-115 > URL: https://issues.apache.org/jira/browse/NUMBERS-115 > Project: Commons Numbers > Issue Type: Task >Reporter: Heinrich Bohne >Priority: Minor > > As has been > [discussed|http://mail-archives.apache.org/mod_mbox/commons-dev/201905.mbox/%3C8883d6fc-2a22-608a-af3d-96272cd425c9%40gmx.at%3E] > on the developers' mailing list, the whole project can be migrated to JUnit > 5. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Created] (NUMBERS-115) Migrate to JUnit 5
Heinrich Bohne created NUMBERS-115: -- Summary: Migrate to JUnit 5 Key: NUMBERS-115 URL: https://issues.apache.org/jira/browse/NUMBERS-115 Project: Commons Numbers Issue Type: Task Reporter: Heinrich Bohne As has been [discussed|http://mail-archives.apache.org/mod_mbox/commons-dev/201905.mbox/%3C8883d6fc-2a22-608a-af3d-96272cd425c9%40gmx.at%3E] on the developers' mailing list, the whole project can be migrated to JUnit 5. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (NUMBERS-100) Code in file FractionTest.java is unsatisfactory
[
https://issues.apache.org/jira/browse/NUMBERS-100?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16862066#comment-16862066
]
Heinrich Bohne commented on NUMBERS-100:
{quote}Is this also how the rest of the test code in [Numbers] has been
modified?
{quote}
No, I only added the code blocks to {{FractionTest}} in this pull request.
Since this has nothing to do with migrating to JUnit 5, nothing of this sort
has been done in the migration branch.
{quote}I'm not sold to using code blocks here.
{quote}
Do you mean, only in the method you quoted, or in the whole class in general?
If it's the former, it might have something to do with the fact that, in this
example, there are three distinct variables, whereas in other methods (like
{{testReciprocal()}}, {{testNegate()}} or {{testAdd()}}), some variables are
used throughout the code.
I will post this to the mailing list as you suggested.
> Code in file FractionTest.java is unsatisfactory
>
>
> Key: NUMBERS-100
> URL: https://issues.apache.org/jira/browse/NUMBERS-100
> Project: Commons Numbers
> Issue Type: Improvement
> Components: fraction
>Reporter: Heinrich Bohne
>Priority: Trivial
> Time Spent: 1h 10m
> Remaining Estimate: 0h
>
> The following characteristics of the file {{FractionTest.java}} can be
> improved:
> * The second-to-last try-catch-block in the method {{testAdd()}} is a
> duplicate of the preceding try-catch-block and is therefore redundant.
> * In the method {{testPow()}}, the conditions {{assertFraction(9, 49,
> a.pow(2))}} and {{assertFraction(49, 9, a.pow(-2))}} are tested twice each
> (once in the block after {{a}}'s declaration, and a second time in the block
> after {{b}}'s declaration. This is probably a typo.
> * The last two assertions in the method {{testGetReducedFraction()}} pass the
> parameters to the method {{Assert.assertEquals(long, long)}} in the wrong
> order (the expected value should go first).
> * Several methods in this class contain a number of tests that use shared
> local variables but are completely independent of each other because these
> local variables get assigned new values at the beginning of a test. The fact
> that the scope of these local variables encompasses all those independent
> tests makes the code look more confusing than necessary.
> * Except for the method {{testGoldenRatio()}}, the throwing of an exception
> is tested with a construct involving the swallowing of an exception, rather
> than an explicit syntax.
> * The helper method {{assertFraction(int, int, Fraction)}} is neglected
> throughout large sections of the class in favor of
> {{Assert.assertEquals(long, long)}} pairs, increasing the amount of code
> duplication.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (NUMBERS-100) Code in file FractionTest.java is unsatisfactory
[
https://issues.apache.org/jira/browse/NUMBERS-100?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16861026#comment-16861026
]
Heinrich Bohne commented on NUMBERS-100:
Understandable. The pull request I was referencing is
[#36|https://github.com/apache/commons-numbers/pull/36], which is currently the
only active pull request related to this JIRA ticket. It addresses all issues
listed in this ticket except for the exception testing syntax involving
swallowing exceptions. My intended resolution of this issue (i.e. using
assertThrows) requires JUnit 5.
So my original intention was to first get pull request
[#36|https://github.com/apache/commons-numbers/pull/36] merged and then migrate
the class {{FractionTest}} to JUnit 5, because pull request #36 predates the
work related to the migration to JUnit 5, and first migrating the project to
JUnit 5 and then rebasing PR #36 would therefore have meant considerably more
work than first merging #36 and then migrating {{FractionTest}} from scratch.
And considering that the merge of fraction-dev had already introduced
conflicts, I was not keen on rebasing the PR a second time.
Since grimreaper offered his cooperation in the migration process, I [alerted
him of
this|https://github.com/apache/commons-numbers/pull/48#issuecomment-500208978],
and he
[agreed|https://github.com/apache/commons-numbers/pull/48#issuecomment-500245575]
to wait with the migration of the fraction module until PR #36 is merged (or
otherwise closed unmerged, in case it is rejected) and [revoked his
request|https://github.com/apache/commons-numbers/pull/36?_pjax=%23js-repo-pjax-container#issuecomment-499046333]
that PR #36 be delayed until the migration to JUnit 5 is complete.
But yesterday, grimreaper completely migrated the class {{FractionTest}} to
JUnit 5 in our migration branch anyway, thereby invalidating even more work put
into the PR considering that I had already updated the PR to be compatible with
the merge of fraction-dev.
So currently, I am a bit at a loss as to what to do. I already [pointed the
above problem
out|https://github.com/apache/commons-numbers/pull/48#issuecomment-500389290]
to grimreaper, but he hasn't yet responded. I don't think that I did anything
wrong, and would consider it justified if #36 is merged, but this would
invalidate grimreaper's changes in the migration branch (which he performed
despite our agreement), and I don't want to cause conflict.
> Code in file FractionTest.java is unsatisfactory
>
>
> Key: NUMBERS-100
> URL: https://issues.apache.org/jira/browse/NUMBERS-100
> Project: Commons Numbers
> Issue Type: Improvement
> Components: fraction
>Reporter: Heinrich Bohne
>Priority: Trivial
> Time Spent: 1h 10m
> Remaining Estimate: 0h
>
> The following characteristics of the file {{FractionTest.java}} can be
> improved:
> * The second-to-last try-catch-block in the method {{testAdd()}} is a
> duplicate of the preceding try-catch-block and is therefore redundant.
> * In the method {{testPow()}}, the conditions {{assertFraction(9, 49,
> a.pow(2))}} and {{assertFraction(49, 9, a.pow(-2))}} are tested twice each
> (once in the block after {{a}}'s declaration, and a second time in the block
> after {{b}}'s declaration. This is probably a typo.
> * The last two assertions in the method {{testGetReducedFraction()}} pass the
> parameters to the method {{Assert.assertEquals(long, long)}} in the wrong
> order (the expected value should go first).
> * Several methods in this class contain a number of tests that use shared
> local variables but are completely independent of each other because these
> local variables get assigned new values at the beginning of a test. The fact
> that the scope of these local variables encompasses all those independent
> tests makes the code look more confusing than necessary.
> * Except for the method {{testGoldenRatio()}}, the throwing of an exception
> is tested with a construct involving the swallowing of an exception, rather
> than an explicit syntax.
> * The helper method {{assertFraction(int, int, Fraction)}} is neglected
> throughout large sections of the class in favor of
> {{Assert.assertEquals(long, long)}} pairs, increasing the amount of code
> duplication.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (NUMBERS-100) Code in file FractionTest.java is unsatisfactory
[
https://issues.apache.org/jira/browse/NUMBERS-100?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16860698#comment-16860698
]
Heinrich Bohne commented on NUMBERS-100:
I've updated the pull request and resolved the conflicts with branch 'master'
that were previously present due to the merge of the branch 'fraction-dev'.
> Code in file FractionTest.java is unsatisfactory
>
>
> Key: NUMBERS-100
> URL: https://issues.apache.org/jira/browse/NUMBERS-100
> Project: Commons Numbers
> Issue Type: Improvement
> Components: fraction
>Reporter: Heinrich Bohne
>Priority: Trivial
> Time Spent: 1h 10m
> Remaining Estimate: 0h
>
> The following characteristics of the file {{FractionTest.java}} can be
> improved:
> * The second-to-last try-catch-block in the method {{testAdd()}} is a
> duplicate of the preceding try-catch-block and is therefore redundant.
> * In the method {{testPow()}}, the conditions {{assertFraction(9, 49,
> a.pow(2))}} and {{assertFraction(49, 9, a.pow(-2))}} are tested twice each
> (once in the block after {{a}}'s declaration, and a second time in the block
> after {{b}}'s declaration. This is probably a typo.
> * The last two assertions in the method {{testGetReducedFraction()}} pass the
> parameters to the method {{Assert.assertEquals(long, long)}} in the wrong
> order (the expected value should go first).
> * Several methods in this class contain a number of tests that use shared
> local variables but are completely independent of each other because these
> local variables get assigned new values at the beginning of a test. The fact
> that the scope of these local variables encompasses all those independent
> tests makes the code look more confusing than necessary.
> * Except for the method {{testGoldenRatio()}}, the throwing of an exception
> is tested with a construct involving the swallowing of an exception, rather
> than an explicit syntax.
> * The helper method {{assertFraction(int, int, Fraction)}} is neglected
> throughout large sections of the class in favor of
> {{Assert.assertEquals(long, long)}} pairs, increasing the amount of code
> duplication.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (NUMBERS-114) Class LinearCombinationTest in arrays module does not compile
[
https://issues.apache.org/jira/browse/NUMBERS-114?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16859026#comment-16859026
]
Heinrich Bohne commented on NUMBERS-114:
Thank you. BTW, I think I figured out why Maven was able to build the module. I
ran {{mvn clean test}} from {{commons-numbers-arrays}} while the build of
{{commons-numbers-fraction}} was still stuck at an older revision before the
merge of fraction-dev, so the obsolete constructors invoked from the test class
in {{commons-numbers-arrays}} were still present in the .class files of the
{{commons-numbers-fraction}} build, even if they did not exist anymore in the
source files.
> Class LinearCombinationTest in arrays module does not compile
> -
>
> Key: NUMBERS-114
> URL: https://issues.apache.org/jira/browse/NUMBERS-114
> Project: Commons Numbers
> Issue Type: Bug
>Reporter: Heinrich Bohne
>Priority: Major
>
> The recent merge of the fraction-dev branch into the master branch broke the
> class {{LinearCombinationTest}} in the arrays module. This class accesses a
> constructor {{BigFraction(long, long)}}, which was available before the merge
> but is no longer so.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Closed] (NUMBERS-114) Class LinearCombinationTest in arrays module does not compile
[
https://issues.apache.org/jira/browse/NUMBERS-114?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Heinrich Bohne closed NUMBERS-114.
--
Resolution: Fixed
> Class LinearCombinationTest in arrays module does not compile
> -
>
> Key: NUMBERS-114
> URL: https://issues.apache.org/jira/browse/NUMBERS-114
> Project: Commons Numbers
> Issue Type: Bug
>Reporter: Heinrich Bohne
>Priority: Major
>
> The recent merge of the fraction-dev branch into the master branch broke the
> class {{LinearCombinationTest}} in the arrays module. This class accesses a
> constructor {{BigFraction(long, long)}}, which was available before the merge
> but is no longer so.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (NUMBERS-114) Class LinearCombinationTest in arrays module does not compile
[
https://issues.apache.org/jira/browse/NUMBERS-114?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16858982#comment-16858982
]
Heinrich Bohne commented on NUMBERS-114:
This is very curious, because Maven does not complain about this when trying to
run the tests. Apparently, it both automatically converts the passed {{long}}
parameters to {{BigInteger}}, and it also does not care about the fact that the
constructor {{BigFraction(BigInteger, BigInteger)}} is declared {{private}} and
should therefore be out of scope. Can anyone shed light on the reason for this
behavior? Is this a bug in Maven?
> Class LinearCombinationTest in arrays module does not compile
> -
>
> Key: NUMBERS-114
> URL: https://issues.apache.org/jira/browse/NUMBERS-114
> Project: Commons Numbers
> Issue Type: Bug
>Reporter: Heinrich Bohne
>Priority: Major
>
> The recent merge of the fraction-dev branch into the master branch broke the
> class {{LinearCombinationTest}} in the arrays module. This class accesses a
> constructor {{BigFraction(long, long)}}, which was available before the merge
> but is no longer so.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Created] (NUMBERS-114) Class LinearCombinationTest in arrays module does not compile
Heinrich Bohne created NUMBERS-114:
--
Summary: Class LinearCombinationTest in arrays module does not
compile
Key: NUMBERS-114
URL: https://issues.apache.org/jira/browse/NUMBERS-114
Project: Commons Numbers
Issue Type: Bug
Reporter: Heinrich Bohne
The recent merge of the fraction-dev branch into the master branch broke the
class {{LinearCombinationTest}} in the arrays module. This class accesses a
constructor {{BigFraction(long, long)}}, which was available before the merge
but is no longer so.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Closed] (NUMBERS-113) Broken/deprecated/old JUnit dependencies in pom.xml
[
https://issues.apache.org/jira/browse/NUMBERS-113?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Heinrich Bohne closed NUMBERS-113.
--
Resolution: Fixed
> Broken/deprecated/old JUnit dependencies in pom.xml
> ---
>
> Key: NUMBERS-113
> URL: https://issues.apache.org/jira/browse/NUMBERS-113
> Project: Commons Numbers
> Issue Type: Bug
>Reporter: Heinrich Bohne
>Priority: Major
> Time Spent: 1h 20m
> Remaining Estimate: 0h
>
> The recent
> [commit|https://github.com/apache/commons-numbers/commit/35b53cff1a9fc8c9a5b4a44bd33d22c959c4]
> where JUnit 5 was introduced seems to have broken something. Apparently, the
> version number for the junit-platform-runner dependency (version 5.0.0-M4)
> does not exist.
> Also, the latest stable version of junit-jupiter-engine is 5.4.2, but the
> pom.xml declares a version number 5.0.0-M4 also for this dependency.
> Besides, I'm getting a warning when trying to run tests:
> bq. The junit-platform-surefire-provider has been deprecated and is scheduled
> to be removed in JUnit Platform 1.4. Please use the built-in support in Maven
> Surefire >= 2.22.0 instead.
> I'm not sure what this means, but I assume it has something to do with the
> dependencies configured for the surefire plugin, which were not there before.
> Besides, I think the appropriate scope for the junit-jupiter-engine
> dependency is {{test}} (currently, it is not set, so it defaults to
> {{compile}}).
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Created] (NUMBERS-113) Broken/deprecated/old JUnit dependencies in pom.xml
Heinrich Bohne created NUMBERS-113:
--
Summary: Broken/deprecated/old JUnit dependencies in pom.xml
Key: NUMBERS-113
URL: https://issues.apache.org/jira/browse/NUMBERS-113
Project: Commons Numbers
Issue Type: Bug
Reporter: Heinrich Bohne
The recent
[commit|https://github.com/apache/commons-numbers/commit/35b53cff1a9fc8c9a5b4a44bd33d22c959c4]
where JUnit 5 was introduced seems to have broken something. Apparently, the
version number for the junit-platform-runner dependency (version 5.0.0-M4) does
not exist.
Also, the latest stable version of junit-jupiter-engine is 5.4.2, but the
pom.xml declares a version number 5.0.0-M4 also for this dependency.
Besides, I'm getting a warning when trying to run tests:
bq. The junit-platform-surefire-provider has been deprecated and is scheduled
to be removed in JUnit Platform 1.4. Please use the built-in support in Maven
Surefire >= 2.22.0 instead.
I'm not sure what this means, but I assume it has something to do with the
dependencies configured for the surefire plugin, which were not there before.
Besides, I think the appropriate scope for the junit-jupiter-engine dependency
is {{test}} (currently, it is not set, so it defaults to {{compile}}).
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Closed] (NUMBERS-104) Speed up trial division
[
https://issues.apache.org/jira/browse/NUMBERS-104?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Heinrich Bohne closed NUMBERS-104.
--
Resolution: Fixed
> Speed up trial division
> ---
>
> Key: NUMBERS-104
> URL: https://issues.apache.org/jira/browse/NUMBERS-104
> Project: Commons Numbers
> Issue Type: Improvement
> Components: primes
>Reporter: Heinrich Bohne
>Priority: Minor
> Time Spent: 50m
> Remaining Estimate: 0h
>
> Currently, the method {{SmallPrimes.boundedTrialDivision(int, int,
> List)}} skips multiples of 2 and 3, thereby reducing the amount of
> integers to be tried to 1/3 of all integers. This can be improved at the cost
> of extra memory by extending the set of prime factors to be skipped in the
> trial division, for example, to 2, 3, 5, 7, and 11.
> However, instead of code duplication, which is the way the code currently
> achieves this, a way to implement this could be:
> * First, precompute and store all integers between 0 (inclusive) and
> 2⋅3⋅5⋅7⋅11=2310 (exclusive) that are not divisible by any of those 5 prime
> numbers (480 numbers in total, according to my experiments).
> * Then, when performing the trial division, one only needs to try out numbers
> of the form 2310⋅k + c, where k is a non-negative integer and c is one of the
> 480 numbers described in the previous step.
> That way, the amount of integers to be tried will be 480/2310 ≈ 20.78%,
> instead of 1/3 ≈ 33.33%, which is a speedup of about 60.42% compared to the
> current algorithm. Of course, with more prime factors eliminated, less
> numbers will have to be tried, but it seems that the returns are quickly
> diminishing compared to the required memory. For example, when also
> eliminating the prime factors 13, 17 and 19, the memory required increases to
> 1658880 integers, whereas the percentage of integers to be tried only drops
> to about 17.10%.
> Since the class {{SmallPrimes}} already stores an array containing the first
> 512 prime numbers, a 480-element {{int}} array seems like a reasonable size
> and a small price to pay for a 60.42% speedup.
> I'm just not entirely sure whether this should go here or on the developer
> mailing list. Since this is actually not a new idea, but just an enhancement
> of an already existing idea, I'm putting it here, but on the other hand, it
> requires some re-writing and some new code, so maybe I'm wrong.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Comment Edited] (NUMBERS-104) Speed up trial division
[
https://issues.apache.org/jira/browse/NUMBERS-104?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16855187#comment-16855187
]
Heinrich Bohne edited comment on NUMBERS-104 at 6/4/19 2:06 AM:
OK, done. The last {{if (n != 1)}} condition is still not fully covered, but
this is because {{n != 1}} will only evaluate to false here if, all other
conditions the method's contract imposes being fulfilled, the method is called
with an unnecessarily high value for the parameter {{maxFactor}}, or, more
specifically, if the parameter {{maxFactor}} is greater than or equal to the
parameter {{n}}, which is an indication of a design flaw of the method itself
rather than the unit tests, and this design flaw is outside the scope of this
ticket.
was (Author: schamschi):
OK, done. The last {{if (n != 1)}} condition is still not fully covered, but
this is because {{n != 1}} will only evaluate to false here if, all other
conditions the method's contract imposes being fulfilled, the method is called
with an unnecessarily high value for the parameter {{maxFactor}}, or, more
specifically, if the parameters {{maxFactor}} and {{n}} are identical, which is
an indication of a design flaw of the method itself rather than the unit tests,
and this design flaw is outside the scope of this ticket.
> Speed up trial division
> ---
>
> Key: NUMBERS-104
> URL: https://issues.apache.org/jira/browse/NUMBERS-104
> Project: Commons Numbers
> Issue Type: Improvement
> Components: primes
>Reporter: Heinrich Bohne
>Priority: Minor
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> Currently, the method {{SmallPrimes.boundedTrialDivision(int, int,
> List)}} skips multiples of 2 and 3, thereby reducing the amount of
> integers to be tried to 1/3 of all integers. This can be improved at the cost
> of extra memory by extending the set of prime factors to be skipped in the
> trial division, for example, to 2, 3, 5, 7, and 11.
> However, instead of code duplication, which is the way the code currently
> achieves this, a way to implement this could be:
> * First, precompute and store all integers between 0 (inclusive) and
> 2⋅3⋅5⋅7⋅11=2310 (exclusive) that are not divisible by any of those 5 prime
> numbers (480 numbers in total, according to my experiments).
> * Then, when performing the trial division, one only needs to try out numbers
> of the form 2310⋅k + c, where k is a non-negative integer and c is one of the
> 480 numbers described in the previous step.
> That way, the amount of integers to be tried will be 480/2310 ≈ 20.78%,
> instead of 1/3 ≈ 33.33%, which is a speedup of about 60.42% compared to the
> current algorithm. Of course, with more prime factors eliminated, less
> numbers will have to be tried, but it seems that the returns are quickly
> diminishing compared to the required memory. For example, when also
> eliminating the prime factors 13, 17 and 19, the memory required increases to
> 1658880 integers, whereas the percentage of integers to be tried only drops
> to about 17.10%.
> Since the class {{SmallPrimes}} already stores an array containing the first
> 512 prime numbers, a 480-element {{int}} array seems like a reasonable size
> and a small price to pay for a 60.42% speedup.
> I'm just not entirely sure whether this should go here or on the developer
> mailing list. Since this is actually not a new idea, but just an enhancement
> of an already existing idea, I'm putting it here, but on the other hand, it
> requires some re-writing and some new code, so maybe I'm wrong.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (NUMBERS-104) Speed up trial division
[
https://issues.apache.org/jira/browse/NUMBERS-104?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16855187#comment-16855187
]
Heinrich Bohne commented on NUMBERS-104:
OK, done. The last {{if (n != 1)}} condition is still not fully covered, but
this is because {{n != 1}} will only evaluate to false here if, all other
conditions the method's contract imposes being fulfilled, the method is called
with an unnecessarily high value for the parameter {{maxFactor}}, or, more
specifically, if the parameters {{maxFactor}} and {{n}} are identical, which is
an indication of a design flaw of the method itself rather than the unit tests,
and this design flaw is outside the scope of this ticket.
> Speed up trial division
> ---
>
> Key: NUMBERS-104
> URL: https://issues.apache.org/jira/browse/NUMBERS-104
> Project: Commons Numbers
> Issue Type: Improvement
> Components: primes
>Reporter: Heinrich Bohne
>Priority: Minor
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> Currently, the method {{SmallPrimes.boundedTrialDivision(int, int,
> List)}} skips multiples of 2 and 3, thereby reducing the amount of
> integers to be tried to 1/3 of all integers. This can be improved at the cost
> of extra memory by extending the set of prime factors to be skipped in the
> trial division, for example, to 2, 3, 5, 7, and 11.
> However, instead of code duplication, which is the way the code currently
> achieves this, a way to implement this could be:
> * First, precompute and store all integers between 0 (inclusive) and
> 2⋅3⋅5⋅7⋅11=2310 (exclusive) that are not divisible by any of those 5 prime
> numbers (480 numbers in total, according to my experiments).
> * Then, when performing the trial division, one only needs to try out numbers
> of the form 2310⋅k + c, where k is a non-negative integer and c is one of the
> 480 numbers described in the previous step.
> That way, the amount of integers to be tried will be 480/2310 ≈ 20.78%,
> instead of 1/3 ≈ 33.33%, which is a speedup of about 60.42% compared to the
> current algorithm. Of course, with more prime factors eliminated, less
> numbers will have to be tried, but it seems that the returns are quickly
> diminishing compared to the required memory. For example, when also
> eliminating the prime factors 13, 17 and 19, the memory required increases to
> 1658880 integers, whereas the percentage of integers to be tried only drops
> to about 17.10%.
> Since the class {{SmallPrimes}} already stores an array containing the first
> 512 prime numbers, a 480-element {{int}} array seems like a reasonable size
> and a small price to pay for a 60.42% speedup.
> I'm just not entirely sure whether this should go here or on the developer
> mailing list. Since this is actually not a new idea, but just an enhancement
> of an already existing idea, I'm putting it here, but on the other hand, it
> requires some re-writing and some new code, so maybe I'm wrong.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Closed] (MATH-1484) Fraction.add(int) and Fraction.subtract(int) ignore risk of integer overflow
[
https://issues.apache.org/jira/browse/MATH-1484?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Heinrich Bohne closed MATH-1484.
Resolution: Won't Fix
> Fraction.add(int) and Fraction.subtract(int) ignore risk of integer overflow
>
>
> Key: MATH-1484
> URL: https://issues.apache.org/jira/browse/MATH-1484
> Project: Commons Math
> Issue Type: Bug
>Affects Versions: 3.6.1
>Reporter: Heinrich Bohne
>Priority: Minor
>
> The methods {{add(int)}} and {{subtract(int)}} in the class
> {{org.apache.commons.math4.fraction.Fraction.java}} do not take into account
> the risk of an integer overflow. For example, (2^31^ - 1)/2 + 1 = (2^31^ +
> 1)/2, so the numerator overflows an {{int}}, but when calculated with
> {{Fraction.add(int)}}, the method still returns normally.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Comment Edited] (NUMBERS-79) Convert fraction addition from BigInteger-based to long-based
[
https://issues.apache.org/jira/browse/NUMBERS-79?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16852413#comment-16852413
]
Heinrich Bohne edited comment on NUMBERS-79 at 5/30/19 10:33 PM:
-
I found this ticket by accident, and it made me think. And the more I have
thought about it, the more convinced I am that the calculation where the
original code claims to require 65 bits of precision can, in fact, _always_ be
performed using only {{long}} values without causing overflow. First, to
recollect the problem at hand:
The objective is to calculate ^m^⁄~a~ + ^n^⁄~b~, where m is coprime to a and n
is coprime to b. Now, let d be the greatest common divisor of a and b, then the
calculation becomes
^m^⁄~p⋅d~ + ^n^⁄~q⋅d~
, where p and q are coprime, and m is coprime to p and d, and n is coprime to q
and d. Continuing this, we get:
^m^⁄~p⋅d~ + ^n^⁄~q⋅d~
= ^m⋅q^⁄~p⋅q⋅d~ + ^n⋅p^⁄~p⋅q⋅d~
= ^m⋅q + n⋅p^⁄~p⋅q⋅d~
The variables m, n, p and q can all be represented as an {{int}}, therefore,
the terms m⋅q and n⋅p can each be represented as a {{long}}. The question is,
can m⋅q + n⋅p be represented as a {{long}}? To examine this, let's consider the
worst case scenario, where every one of these 4 variables has the largest
possible absolute value an {{int}} can have, which is 2^31^. This is only
possible if the {{int}} value is negative, i.e. -2^31^. So then, our numerator
term becomes:
m⋅q + n⋅p
= (-2^31^) ⋅ (-2^31^) + (-2^31^) ⋅ (-2^31^)
= 2^63^
, which exceeds the largest possible {{long}} value only by one. However, this
scenario will never arise, because m and p are coprime, just like n and q are
coprime and q and p are coprime. So these 4 variables cannot all be -2^31^,
which means that the absolute value of the term m⋅q + n⋅p _must_ be smaller
than 2^63^ and therefore representable as a {{long}}.
Perhaps the source the code comments refer to is talking about unsigned
integers, in which case 64 bits would indeed not be enough, as far as I can
tell.
However, the current version in the fraction-dev branch, which, I assume,
contains the fix to this ticket's issue, restricts even the intermediate values
in the above calculation to {{int}} instead of {{long}} type, which would be
what this ticket suggests. This creates the problem of throwing an exception
even if the final value can be represented as an {{int}}, because, while m⋅q +
n⋅p is definitely coprime to p and q, it could potentially share a factor with
d, so even if m⋅q + n⋅p cannot be represent as an {{int}}, the final numerator
could. For example:
^362,564,597^⁄~10~ + ^274,164,323^⁄~6~ = ^1,229,257,703^⁄~15~
All numerators and denominators of these fractions can be expressed as an
{{int}}, but the current code in fraction-dev fails to calculate this sum,
because it restricts even intermediate values to type {{int}}, and not just the
final values. If it were to store and perform calculations on intermediate
values as {{long}} variables, then it would work.
Since the data type {{long}} will always be sufficient for intermediate values,
thus eliminating the need for {{BigInteger}}, I don't see a reason to use
{{int}} instead of {{long}} values if this could cause the method to fail when
it would otherwise calculate the correct result.
was (Author: schamschi):
I found this ticket by accident, and it made me think. And the more I have
thought about it, the more convinced I am that the calculation where the
original code claims to require 65 bits of precision can, in fact, _always_ be
performed using only {{long}} values without causing overflow. First, to
recollect the problem at hand:
The objective is to calculate ^m^⁄~a~ + ^n^⁄~b~, where m is coprime to a and n
is coprime to b. Now, let d be the greatest common divisor of a and b, then the
calculation becomes
^m^⁄~p⋅d~ + ^n^⁄~q⋅d~
, where p and q are coprime, and m is coprime to p and d, and n is coprime to q
and d. Continuing this, we get:
^m^⁄~p⋅d~ + ^n^⁄~q⋅d~
= ^m⋅q^⁄~p⋅q⋅d~ + ^n⋅p^⁄~p⋅q⋅d~
= ^m⋅q + n⋅p^⁄~p⋅q⋅d~
The variables m, n, p and q can all be represented as an {{int}}, therefore,
the terms m⋅q and n⋅p can each be represented as a {{long}}. The question is,
can m⋅q + n⋅p be represented as a {{long}}? To examine this, let's consider the
worst case scenario, where every one of these 4 variables has the largest
possible absolute value an {{int}} can have, which is 2^31^. This is only
possible if the {{int}} value is negative, i.e. -2^31^. So then, our numerator
term becomes:
m⋅q + n⋅p
= (-2^31^) ⋅ (-2^31^) + (-2^31^) ⋅ (-2^31^)
= 2^63^
, which exceeds the largest possible {{long}} value only by one. However, this
scenario will never arise, because m and p are coprime, just like n and q are
coprime and q and p are coprime. So these 4 variables cannot all be -2^31^,
which means that the absolute value of the term m⋅q + n⋅p
[jira] [Commented] (NUMBERS-79) Convert fraction addition from BigInteger-based to long-based
[
https://issues.apache.org/jira/browse/NUMBERS-79?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16852413#comment-16852413
]
Heinrich Bohne commented on NUMBERS-79:
---
I found this ticket by accident, and it made me think. And the more I have
thought about it, the more convinced I am that the calculation where the
original code claims to require 65 bits of precision can, in fact, _always_ be
performed using only {{long}} values without causing overflow. First, to
recollect the problem at hand:
The objective is to calculate ^m^⁄~a~ + ^n^⁄~b~, where m is coprime to a and n
is coprime to b. Now, let d be the greatest common divisor of a and b, then the
calculation becomes
^m^⁄~p⋅d~ + ^n^⁄~q⋅d~
, where p and q are coprime, and m is coprime to p and d, and n is coprime to q
and d. Continuing this, we get:
^m^⁄~p⋅d~ + ^n^⁄~q⋅d~
= ^m⋅q^⁄~p⋅q⋅d~ + ^n⋅p^⁄~p⋅q⋅d~
= ^m⋅q + n⋅p^⁄~p⋅q⋅d~
The variables m, n, p and q can all be represented as an {{int}}, therefore,
the terms m⋅q and n⋅p can each be represented as a {{long}}. The question is,
can m⋅q + n⋅p be represented as a {{long}}? To examine this, let's consider the
worst case scenario, where every one of these 4 variables has the largest
possible absolute value an {{int}} can have, which is 2^31^. This is only
possible if the {{int}} value is negative, i.e. -2^31^. So then, our numerator
term becomes:
m⋅q + n⋅p
= (-2^31^) ⋅ (-2^31^) + (-2^31^) ⋅ (-2^31^)
= 2^63^
, which exceeds the largest possible {{long}} value only by one. However, this
scenario will never arise, because m and p are coprime, just like n and q are
coprime and q and p are coprime. So these 4 variables cannot all be -2^31^,
which means that the absolute value of the term m⋅q + n⋅p _must_ be smaller
than 2^63^ and therefore representable as a {{long}}.
Perhaps the source the code comments refer to is talking about unsigned
integers, in which case 64 bits would indeed not be enough, as far as I can
tell.
However, the current version in the fraction-dev branch, which, I assume,
contains the fix to this ticket's issue, restricts even the intermediate values
in the above calculation to {{int}}s instead of {{longs}}, which would be what
this ticket suggests. This creates the problem of throwing an exception even if
the final value can be represented as an {{int}}, because, while m⋅q + n⋅p is
definitely coprime to p and q, it could potentially share a factor with d, so
even if m⋅q + n⋅p cannot be represent as an {{int}}, the final numerator could.
For example:
^362,564,597^⁄~10~ + ^274,164,323^⁄~6~ = ^1,229,257,703^⁄~15~
All numerators and denominators of these fractions can be expressed as an
{{int}}, but the current code in fraction-dev fails to calculate this sum,
because it restricts even intermediate values to {{int}}s, and not just the
final values. If it were to store and perform calculations on intermediate
values as {{long}}s, then it would work.
Since the data type {{long}} will always be sufficient for intermediate values,
thus eliminating the need for {{BigInteger}}s, I don't see a reason to use
{{int}}s instead of {{long}}s if this could cause the method to fail when it
would otherwise calculate the correct result.
> Convert fraction addition from BigInteger-based to long-based
> -
>
> Key: NUMBERS-79
> URL: https://issues.apache.org/jira/browse/NUMBERS-79
> Project: Commons Numbers
> Issue Type: Improvement
>Reporter: Eric Barnhill
>Assignee: Eric Barnhill
>Priority: Minor
>
> Current Fraction class uses BigInteger due to the fact that in extreme cases
> adding and subtraction of fractions can require 65 bits of precision. However
> this is very costly in terms of performance for all but the most extreme
> cases. This ticket proposes using long-based arithmetic for Fraction addition
> and subtraction, and recommending BigFraction if the operation may run very
> large.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Created] (NUMBERS-104) Speed up trial division
Heinrich Bohne created NUMBERS-104:
--
Summary: Speed up trial division
Key: NUMBERS-104
URL: https://issues.apache.org/jira/browse/NUMBERS-104
Project: Commons Numbers
Issue Type: Improvement
Components: primes
Reporter: Heinrich Bohne
Currently, the method {{SmallPrimes.boundedTrialDivision(int, int,
List)}} skips multiples of 2 and 3, thereby reducing the amount of
integers to be tried to 1/3 of all integers. This can be improved at the cost
of extra memory by extending the set of prime factors to be skipped in the
trial division, for example, to 2, 3, 5, 7, and 11.
However, instead of code duplication, which is the way the code currently
achieves this, a way to implement this could be:
* First, precompute and store all integers between 0 (inclusive) and
2⋅3⋅5⋅7⋅11=2310 (exclusive) that are not divisible by any of those 5 prime
numbers (480 numbers in total, according to my experiments).
* Then, when performing the trial division, one only needs to try out numbers
of the form 2310⋅k + c, where k is a non-negative integer and c is one of the
480 numbers described in the previous step.
That way, the amount of integers to be tried will be 480/2310 ≈ 20.78%, instead
of 1/3 ≈ 33.33%, which is a speedup of about 60.42% compared to the current
algorithm. Of course, with more prime factors eliminated, less numbers will
have to be tried, but it seems that the returns are quickly diminishing
compared to the required memory. For example, when also eliminating the prime
factors 13, 17 and 19, the memory required increases to 1658880 integers,
whereas the percentage of integers to be tried only drops to about 17.10%.
Since the class {{SmallPrimes}} already stores an array containing the first
512 prime numbers, a 480-element {{int}} array seems like a reasonable size and
a small price to pay for a 60.42% speedup.
I'm just not entirely sure whether this should go here or on the developer
mailing list. Since this is actually not a new idea, but just an enhancement of
an already existing idea, I'm putting it here, but on the other hand, it
requires some re-writing and some new code, so maybe I'm wrong.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (NUMBERS-100) Code in file FractionTest.java is unsatisfactory
[
https://issues.apache.org/jira/browse/NUMBERS-100?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16845768#comment-16845768
]
Heinrich Bohne commented on NUMBERS-100:
OK, I've closed [the old pull
request|https://github.com/apache/commons-numbers/pull/35] and opened [a new
one|https://github.com/apache/commons-numbers/pull/36] with all the changes
except the introduction of JUnit 5. I will see to the proposal of introducing
JUnit 5 via the developer mailing list later.
> Code in file FractionTest.java is unsatisfactory
>
>
> Key: NUMBERS-100
> URL: https://issues.apache.org/jira/browse/NUMBERS-100
> Project: Commons Numbers
> Issue Type: Improvement
> Components: fraction
>Reporter: Heinrich Bohne
>Priority: Trivial
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> The following characteristics of the file {{FractionTest.java}} can be
> improved:
> * The second-to-last try-catch-block in the method {{testAdd()}} is a
> duplicate of the preceding try-catch-block and is therefore redundant.
> * In the method {{testPow()}}, the conditions {{assertFraction(9, 49,
> a.pow(2))}} and {{assertFraction(49, 9, a.pow(-2))}} are tested twice each
> (once in the block after {{a}}'s declaration, and a second time in the block
> after {{b}}'s declaration. This is probably a typo.
> * The last two assertions in the method {{testGetReducedFraction()}} pass the
> parameters to the method {{Assert.assertEquals(long, long)}} in the wrong
> order (the expected value should go first).
> * Several methods in this class contain a number of tests that use shared
> local variables but are completely independent of each other because these
> local variables get assigned new values at the beginning of a test. The fact
> that the scope of these local variables encompasses all those independent
> tests makes the code look more confusing than necessary.
> * Except for the method {{testGoldenRatio()}}, the throwing of an exception
> is tested with a construct involving the swallowing of an exception, rather
> than an explicit syntax.
> * The helper method {{assertFraction(int, int, Fraction)}} is neglected
> throughout large sections of the class in favor of
> {{Assert.assertEquals(long, long)}} pairs, increasing the amount of code
> duplication.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Created] (NUMBERS-100) Code in file FractionTest.java is unsatisfactory
Heinrich Bohne created NUMBERS-100:
--
Summary: Code in file FractionTest.java is unsatisfactory
Key: NUMBERS-100
URL: https://issues.apache.org/jira/browse/NUMBERS-100
Project: Commons Numbers
Issue Type: Improvement
Components: fraction
Reporter: Heinrich Bohne
The following characteristics of the file {{FractionTest.java}} can be improved:
* The second-to-last try-catch-block in the method {{testAdd()}} is a duplicate
of the preceding try-catch-block and is therefore redundant.
* In the method {{testPow()}}, the conditions {{assertFraction(9, 49,
a.pow(2))}} and {{assertFraction(49, 9, a.pow(-2))}} are tested twice each
(once in the block after {{a}}'s declaration, and a second time in the block
after {{b}}'s declaration. This is probably a typo.
* The last two assertions in the method {{testGetReducedFraction()}} pass the
parameters to the method {{Assert.assertEquals(long, long)}} in the wrong order
(the expected value should go first).
* Several methods in this class contain a number of tests that use shared local
variables but are completely independent of each other because these local
variables get assigned new values at the beginning of a test. The fact that the
scope of these local variables encompasses all those independent tests makes
the code look more confusing than necessary.
* Except for the method {{testGoldenRatio()}}, the throwing of an exception is
tested with a construct involving the swallowing of an exception, rather than
an explicit syntax.
* The helper method {{assertFraction(int, int, Fraction)}} is neglected
throughout large sections of the class in favor of {{Assert.assertEquals(long,
long)}} pairs, increasing the amount of code duplication.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (NUMBERS-99) Fraction.add(int) and Fraction.subtract(int) ignore risk of integer overflow
[
https://issues.apache.org/jira/browse/NUMBERS-99?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16843441#comment-16843441
]
Heinrich Bohne commented on NUMBERS-99:
---
Regarding your point about multiple test methods: Do you mean the fact that
{{FractionTest.testAdd()}} now tests both {{Fraction.add(Fraction)}} and
{{Fraction.add(int)}}, and that it would be more appropriate to create a
separate test method for {{Fraction.add(int)}}? The reason I didn't do it this
way was that the method {{FractionTest.testAdd()}} already contained a section
where {{Fraction.add(int)}} was tested among all the other tests of
{{Fraction.add(Fraction)}}, so I thought the test would belong there.
The same applies to the test where an exception is expected. I tried to follow
the style of the other tests where an exception is expected (which are present
in abundance, and they all swallow the exception without the test method being
annotated as you describe). So if I understand you correctly, the code in the
test class was already not following the guidelines you mentioned here, and
should therefore be rewritten?
> Fraction.add(int) and Fraction.subtract(int) ignore risk of integer overflow
>
>
> Key: NUMBERS-99
> URL: https://issues.apache.org/jira/browse/NUMBERS-99
> Project: Commons Numbers
> Issue Type: Bug
> Components: fraction
>Reporter: Heinrich Bohne
>Priority: Minor
> Time Spent: 10m
> Remaining Estimate: 0h
>
> The methods {{add(int)}} and {{subtract(int)}} in the class
> {{org.apache.commons.numbers.fraction.Fraction}} do not take into account the
> risk of an integer overflow. For example, (2^31^ - 1)/2 + 1 = (2^31^ +
> 1)/2, so the numerator overflows an {{int}}, but when calculated with
> {{Fraction.add(int)}}, the method still returns normally.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Created] (NUMBERS-99) Fraction.add(int) and Fraction.subtract(int) ignore risk of integer overflow
Heinrich Bohne created NUMBERS-99:
-
Summary: Fraction.add(int) and Fraction.subtract(int) ignore risk
of integer overflow
Key: NUMBERS-99
URL: https://issues.apache.org/jira/browse/NUMBERS-99
Project: Commons Numbers
Issue Type: Bug
Components: fraction
Reporter: Heinrich Bohne
The methods {{add(int)}} and {{subtract(int)}} in the class
{{org.apache.commons.numbers.fraction.Fraction}} do not take into account the
risk of an integer overflow. For example, (2^31^ - 1)/2 + 1 = (2^31^ + 1)/2,
so the numerator overflows an {{int}}, but when calculated with
{{Fraction.add(int)}}, the method still returns normally.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (MATH-1484) Fraction.add(int) and Fraction.subtract(int) ignore risk of integer overflow
[
https://issues.apache.org/jira/browse/MATH-1484?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16843240#comment-16843240
]
Heinrich Bohne commented on MATH-1484:
--
OK, I actually had the issue fixed in my fork of the repository, I just didn't
have time to create a pull request when I posted the issue here (it's my first
time on Github, so I would have needed some time to make sure I do it
correctly). Should I still create the pull request for this project? It seems
to me that, if the class exists in this project, there is no reason not to fix
the bug.
> Fraction.add(int) and Fraction.subtract(int) ignore risk of integer overflow
>
>
> Key: MATH-1484
> URL: https://issues.apache.org/jira/browse/MATH-1484
> Project: Commons Math
> Issue Type: Bug
>Affects Versions: 3.6.1
>Reporter: Heinrich Bohne
>Priority: Minor
>
> The methods {{add(int)}} and {{subtract(int)}} in the class
> {{org.apache.commons.math4.fraction.Fraction.java}} do not take into account
> the risk of an integer overflow. For example, (2^31^ - 1)/2 + 1 = (2^31^ +
> 1)/2, so the numerator overflows an {{int}}, but when calculated with
> {{Fraction.add(int)}}, the method still returns normally.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
