> On 9 Apr 2020, at 16:32, Gilles Sadowski <gillese...@gmail.com> wrote:
>
> Le jeu. 9 avr. 2020 à 16:48, Alex Herbert <alex.d.herb...@gmail.com
> <mailto:alex.d.herb...@gmail.com>> a écrit :
>>
>>
>> On 09/04/2020 14:38, Gilles Sadowski wrote:
>>> Le jeu. 9 avr. 2020 à 14:09, Alex Herbert <alex.d.herb...@gmail.com> a
>>> écrit :
>>>>
>>>> On 09/04/2020 12:04, Alex Herbert wrote:
>>>>>>> So if we are to support 0/-1 then I will add this to the standard test
>>>>>>> cases to make sure it is correctly implemented.
>>>> So I did this. No major functionality failures.
>>>>
>>>> However there is a discrepancy between multiply and other functions.
>>>>
>>>> Multiply will return the form 0 / 1 for zero when either argument is zero.
>>>>
>>>>
>>>> Divide will return the current instance if the instance is zero.
>>>>
>>>> Add/subtract will return the current instance if the instance to add is
>>>> zero.
>>>>
>>>> Pow will return the current instance if the instance is zero.
>>>>
>>>>
>>>> So you get this:
>>>>
>>>> 0 / -1 multiply 1 / 3 == 0 / 1
>>>>
>>>>
>>>> 0 / -1 divide 1 / 3 == 0 / -1
>>>>
>>>>
>>>> 0 / -1 add 0 / 1 == 0 / -1
>>>>
>>>> 0 / -1 pow 1 == 0 / -1
>>>>
>>>> 0 / -1 pow 2 == 0 / -1
>>>>
>>>>
>>>> In the later case you may actually expect pow(0 / -1, 2) to return 0 /
>>>> 1. The result would require a change of sign on the denominator when the
>>>> power is even.
>>>>
>>>>
>>>> So do we:
>>>>
>>>> 1. return the canonical form 0 / 1 when the result is zero?
>>>>
>>>> 2. return the appropriate instance that is zero when the result is zero?
>>>>
>>>> 3. maintain the sign as if implemented using the arithmetic on the
>>>> denominator?
>>>>
>>>>
>>>> I do not see a situation where preserving the signedness of the zero
>>>> denominator is important. This rules out 3.
>>> But didn't you imply above that
>>> 0 / -1 pow 2
>>> should return
>>> 0 / 1
>>> ?
>>
>> If you computed pow(2) it as (0 * 0) / (-1 * -1) then the result is 0 /
>> 1. Likewise pow(3) is 0 / -1. But both are zero.
>>
>> If you see the result as zero then the sign of the denominator does not
>> matter.
>>
>> If you want to preserve the sign of the denominator then more work will
>> have to be done in certain methods. I do not think this is important.
>
> So there is no issue that several representations come up
> during computations (?).
I do not think so. At lest I have not found an issue where the computation is
broken.
>
>> Note: This is in contrast to Complex where the sign of zero components
>> of the complex number have to be preserved to ensure compatibility with
>> ISO C99 standards.
>>
>> So do we leave it to produce 0 / -1 or 0 / 1 depending on the
>> computation (as is done currently), fix it to preserve the sign
>> appropriately or fix it to ensure that zero is always a canonical 0 / 1?
>
> Is there any advantage to a single zero representation?
> If not, I'd vote for the simplest code.
>
>>>
>>>> It makes it cleaner in the code to always return Fraction.ZERO when
>>>> operations result in zero. This has memory advantage by allowing the
>>>> garbage collection of fractions that are zero when they go out of scope
>>>> following computations.
>>> +1
>>
>> Returning 0 / 1 for zero when possible is simple to do in the existing
>> methods that detect zero as one of the arguments.
>
> if (this.isZero()) { return ZERO; }
> if (this.isZero()) { return this; }
>
> Is one better than the other?
> If not, I'd vote for the former (referring to allowing garbage collection).
I will make this change as it makes the code more readable IMO. It is very
clear the result is zero.
>
>>>> This is a weak argument as it is unlikely to
>>>> save much memory in common usage. It is also possible to create zeros in
>>>> computation and we do not want to go the route of switching new 0/1
>>>> instances to ZERO.
>>> Why not?
>> It is extra work in the computations since computations do not detect
>> zeros in the result. They detect zeros in the arguments and can fast
>> rack the result.
>
> I thought that we were indeed referring to the above case (this
> vs ZERO).
>
>> For those that actually compute a result then the result would require
>> construction using a factory constructor that will detect and return
>> ZERO if the numerator is zero.
>
> IIUC, this should be avoided, since ZERO will be returned at
> the next opportunity (e.g. if the new instance calls "multiply")
> without incurring any performance cost.
An interesting point. And if the ultimate output for the fraction is conversion
to a double then it would not matter anyway.
So I will check all the conversions work with the 0 / -1 representation of zero.
>
>> This would add another if statement to
>> all usage of the factory constructor and effect performance. However
>> performance impact is not likely to be high (see below).
>>
>> It should solve the previous issue that you can represent zero as 0 /
>> -1. If all computations use a factory constructor then we can make it
>> possible to exclude the 0 / -1 representation completely.
>>
>>>
>>>> The easiest change is to just update multiply to return the appropriate
>>>> zero instance and not use Fraction.ZERO. The classes then are consistent
>>>> across the methods. Return the appropriate fraction instance if it
>>>> matches the result, otherwise compute the result.
>>> I don't follow: How do you match before computing?
>>
>> I missed out that one of the function arguments match the result when
>> zero is involved as an argument. All the arithmetic methods detect zeros
>> in the arguments and quickly return.
>>
>> It would be cleaner to try and enforce a canonical form for ZERO.
>
> From the above development, I was arriving to the other
> conclusion...
>
>> Looking at the code for the standard constructor using Fraction(num,
>> den) there are already a few if statements in there. In most cases it
>> then follows the path using the reduction with the greatest common
>> divisor logic. This is bound to be the largest part of the execution
>> time. Forcing all construction through the of(num, den) factory method
>> and adding another if statement to detect a zero numerator should not
>> impact performance too much. The class can avoid the factory constructor
>> if the value has already been tested and known to be non zero. This is
>> the case for methods that use a numerator and not a denominator (such as
>> pow(int)).
>
> Well, personally, I don't mind, but I had the impression that,
> in "Complex.java" for example, you tended to go the opposite
> way (i.e. call the constructor directly whenever possible).
In Complex the constructor is dumb and so is the factory constructor
ofCartesian(x, y). It simply stores/passes through the values. All the work to
maintain the ISO C99 compliance is done in the methods.
>
>> Given this I am thinking that using ZERO when possible is a better
>> option and avoid 0 / -1.
>
> Hmm, then I'm both +0 and -0 (which is the same, right?)
> on this issue. ;-)
>
>>
>> Also, did you have an opinion on dropping pow(long) and pow(BigInteger)?
>> I do not think that they can compute a value when pow(int) cannot be used.
>
> Isn't there a JSR project to extend the range of "BigInteger"? ;-)
I do not know.
>
> +1 to remove, but perhaps add an explanation in the Javadoc
> for "pow(int)" to the effect that users should request the missing
> methods.
I will remove these pow methods. They can be added back when BigInteger
supports larger numbers. In that case the unit test using
BigFraction.of(2).pow(Integer.MAX_VALUE) will no longer throw an exception
which will notify us that BigFraction can be extended.
I’ll make some changes to return ZERO when possible in methods. When doing this
I will think some more on whether the factory constructor should do so and the
changes that it would involve.
Alex
>
> Best,
> Gilles
>
>>>> The ZERO constant then is reduced to a convenience as for the ONE constant.
>>>>
>>>> So my vote is option 2 for code consistency.
>>>>
>>>>
>>>> Alex
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> <mailto:dev-unsubscr...@commons.apache.org>
> For additional commands, e-mail: dev-h...@commons.apache.org
> <mailto:dev-h...@commons.apache.org>
