[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-swing-dev
Subject: Re: <Swing Dev> [15] RFR JDK-8220811: SpinnerNumberModel floating point rounding issue
From: "Volodin, Vladislav" <vladislav.volodin () sap ! com>
Date: 2020-03-14 20:33:18
Message-ID: 0D81D11D-910C-4B0F-AF02-E3D768F21D6C () sap ! com
[Download RAW message or body]
Hello Pankaj,
I am not the reviewer, but I agree with Jason. To me p1.equals(Double.NaN) looks \
confusing. Because people might think that it will be equivalent to p1 == Double.NaN, \
that will return false, when p1 is also NaN \
(https://stackoverflow.com/questions/8819738/why-does-double-nan-double-nan-return-false).
I prefer to use the dedicated method such as "public static boolean isNaN(double \
v)". It looks self-descriptive.
Meanwhile, I remember you sentence regarding the number of steps:
> double min=-.15,max=0.15,stepsize=.05, the steps is calculated as 5. double \
min=-.15,max=0.20,stepsize=.05, the steps is calculated as 7 instead of 6. I checked \
this part with the code:
Double min = -.15;
Double max = 0.15;
Double stepsize = .05;
Double steps = (max - min) / stepsize;
And I found out that in this case the number of steps will be 5,999999....., but we \
can compensate it with either Math.round, it will return 6, or we can add the \
"epsilon" value to "max", and count the number of steps as it is: Double max = 0.15 \
+ Math.ulp(1.0); Steps count will be 6.00000238418579.
Since there is no value in Double (and probably float) less than Math.ulp (or \
epsilon, if we use this term), it will be probably safe to use my approach. What do \
you think?
Kind regards,
Vlad
On 14.03.20, 15:51, "Pankaj Bansal" <pankaj.b.bansal@oracle.com> wrote:
Hello Jason,
<< I would assume newMinimum.equals(Double.NaN) and newMinimum.equals(Float.NaN) \
should always evaluate to false. It seems to work as expected.
Double p1 = Double.NaN;
Double p2 = 1.0;
System.out.println(p1.equals(Double.NaN)); //prints true
System.out.println(p2.equals(Double.NaN)); //prints false
Regards,
Pankaj
-----Original Message-----
From: Jason Mehrens <jason_mehrens@hotmail.com>
Sent: Saturday, March 14, 2020 8:09 PM
To: Pankaj Bansal <pankaj.b.bansal@oracle.com>; Sergey Bylokhov \
<sergey.bylokhov@oracle.com>; Alexey Ivanov <alexey.ivanov@oracle.com>; Volodin, \
Vladislav <vladislav.volodin@sap.com> Cc: swing-dev@openjdk.java.net
Subject: Re: <Swing Dev> [15] RFR JDK-8220811: SpinnerNumberModel floating point \
rounding issue
Pankaj,
I would assume newMinimum.equals(Double.NaN) and newMinimum.equals(Float.NaN) \
should always evaluate to false. Perhaps you want to use Float.isNaN and \
Double.isNaN instead?
Jason
________________________________________
From: swing-dev <swing-dev-bounces@openjdk.java.net> on behalf of Pankaj Bansal \
<pankaj.b.bansal@oracle.com> Sent: Saturday, March 14, 2020 8:11 AM
To: Sergey Bylokhov; Alexey Ivanov; Volodin, Vladislav
Cc: swing-dev@openjdk.java.net
Subject: Re: <Swing Dev> [15] RFR JDK-8220811: SpinnerNumberModel floating point \
rounding issue
Hello Sergey,
<< It will differ for two cases:
- The error will not be accumulated when the counter will move \
forward/backward, currently the result might different on each \
iteration.
- Initial/Default value will never be skipped due to counter=0
I tried to code according to my understanding of the idea. I have created a \
preliminary webrev to demonstrate what I am doing. This is nowhere final, so please \
ignore the optimizations. Please have a look. As I was thinking, the precision error \
is creating issue while creating the step count. I have to do lot of stuff to allow \
values to be changed by editing the textfield. There are some other issues also, like \
the double value is formatted according to the formatter and that is also causing \
problems.
webrev: http://cr.openjdk.java.net/~pbansal/8220811/webrev01/
Regards,
Pankaj
-----Original Message-----
From: Sergey Bylokhov
Sent: Wednesday, March 11, 2020 5:33 AM
To: Pankaj Bansal <pankaj.b.bansal@oracle.com>; Alexey Ivanov \
<alexey.ivanov@oracle.com>; Volodin, Vladislav <vladislav.volodin@sap.com> Cc: \
swing-dev@openjdk.java.net Subject: Re: <Swing Dev> [15] RFR JDK-8220811: \
SpinnerNumberModel floating point rounding issue
On 3/10/20 5:34 am, Pankaj Bansal wrote:
> Hello Sergey/Vlad/Alexey,
>
> Sorry, I could not reply to this earlier. I have one doubt about this approach. \
Won't the calculation of stepCount itself suffer from floating point issue? I mean \
the user will pass min, max, stepsize, then wont the calculation of steps required to \
go from min to max will also suffer from same floating point issue? I think there can \
be an rounding of error of -1 or +1 in calculation of step count.
It will differ for two cases:
- The error will not be accumulated when the counter will move \
forward/backward, currently the result might different on each \
iteration.
- Initial/Default value will never be skipped due to counter=0
>
> eg.
>
> int steps =0;
>
> for (double i=min+stepsize; i<=max; i+=stepsize)
> steps++;
>
> double min=-.15,max=0.15,stepsize=.05, the steps is calculated as 5. double \
min=-.15,max=0.20,stepsize=.05, the steps is calculated as 7 instead of 6. >
>
> The reason is that, there is floating point error in first case, but it is not \
present in second case. >
> I think the best we can do here is as Sergey suggested in his first
> reply to use Math.fma to reduce the floating point error chances from
> 2 to 1 or just close this as not an issue
>
> Regards,
>
> Pankaj
>
>
> On 19/02/20 3:49 AM, Sergey Bylokhov wrote:
>> I think it should work, the step will counts from the default value.
>>
>> So currently:
>> 1. if the user set default value to X1 and then he iterates forward 100 times \
then he will get some X2. During this calculation, he could get "100" rounding \
issues. >> 2. If later the user decides iterates backward then most probably he will \
not get X1, and the amount of possible "rounding issues" will be 200. >>
>> If the user will repeat steps 1. and 2. then each time the values will \
"float". >>
>> If we will use counter then in the worst case we will get only two roundings \
per step: X1+step*100 = X2(if we will use fma we will get only one for every step). \
>> >> It will not solve all issues but at least will make the iteration "stable".
>>
>> On 2/17/20 1:59 am, Alexey Ivanov wrote:
>>> Hi Vlad,
>>>
>>> The idea looks reasonable. However, it does not allow for manual editing. The \
cases where max and min values are not multiples of step would be hard to handle with \
this approach. For example: max = 10.05, min = 0.01, step = 0.1; how many ticks are \
there? What if the user enters 1.01015; the value should change to 1.11015 or \
0.91015. >>>
>>> On 13/02/2020 22:22, Volodin, Vladislav wrote:
>>>> Hello Sergey, Alexey and Pankaj,
>>>>
>>>> I am reading the current discussion and I was thinking about an idea \
changing the code in the way that instead of working with float/double numbers we \
work with integer ticks. For example, the model remembers the min/max/step values and \
calculates a number of steps required to reach from min to max. All \
increment/decrement actions are done against the current ˋtickˋ value. If the \
current ˋtickˋ reaches 0 - we return min; if maxTick — we return max. And the \
current value can be always counted as (min + tick * step) if tick is neither zero, \
nor max tick count. >>>>
>>>> At least if we deal with integer ticks, but all reading operations calculate \
on the fly, we will be able to control the representativeness of output. >>>>
>>>> As always, I don't know all the details and possible consequences, so feel \
free to ignore my email, if I am wrong. >>>>
>>>> Kind regards,
>>>> Vlad
>>>>
>>>> Sent from myPad
>>>>
>>>>> On 13. Feb 2020, at 22:34, Sergey Bylokhov <Sergey.Bylokhov@oracle.com> \
wrote: >>>>>
>>>>> On 2/12/20 8:21 am, Alexey Ivanov wrote:
>>>>>> The bug report says that going from -0.15 to -0.10 does not allow going \
back to -0.15. This happens because the result of this sequence of operations cannot \
be represented exactly, or, in other words, because of rounding errors; or rather the \
result is less than the set minimal value. >>>>>> Can we set the value of the \
spinner to the set minimal value instead of disallowing the operation. I mean, after \
going up the displayed value is -0.10; going down by 0.05 gives the result which is \
less than the minimal value for the spinner, and thus going down is not allowed. What \
if we set the value of the spinner to its minimal value instead? >>>>> In this case, \
we will need to update all types including int. >>>>> Isn't it will be surprised \
that the spinner will show the value which is not calculated as "defaultValue + \
stepValue * stepCount"? >>>>>
>>>>>
>>>>> --
>>>>> Best regards, Sergey.
>>
>>
>
--
Best regards, Sergey.
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic