You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by venkatesha murthy <ve...@gmail.com> on 2014/06/23 20:36:16 UTC
[MATH-1130] Patch on canequal (math-1130-canequal.patch) to make
faster equals check with nan
I am adding a canEqual method in MathUtils since neither Math.equals nor
Precision.equalsIncludingNaN performs better under heavy load (I have
attached the test code for the same in the patch).
The reason as far as i can think is :
In case of equals it keeps churning new objects and makes Double.equals
and in case of Precision.equalsIncludingNaN it might be that
it has not covered the case of quick elimination of detecting one of
them as nan as in that case it can return false.
The change i am bringing in is simple in that in any case of either of
them nan or both then return true or false and if and only if both are
*not nan* then get to Precision.equals
Does any on have a concern adding this method or if the change needs
to be done else where such as in Precision.equalsIncludingNaN
Please opinionate.
thanks
venkat
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [MATH-1130] Patch on canequal (math-1130-canequal.patch) to make
faster equals check with nan
Posted by venkatesha murthy <ve...@gmail.com>.
On Wed, Jun 25, 2014 at 7:16 PM, Gilles <gi...@harfang.homelinux.org>
wrote:
> Hi.
>
> [...]
>>>>>
>>>>>
>>>>> The change i am bringing in is simple in that in any case of either of
>>>>> them nan or both then return true or false and if and only if both are
>>>>> *not nan* then get to Precision.equals
>>>>>
>>>>> Does any on have a concern adding this method or if the change needs
>>>>> to be done else where such as in Precision.equalsIncludingNaN
>>>>>
>>>>
>>>>
>>>> What is the semantics of this new method?
>>>> Is it different from all of the following:
>>>> * MathUtils.equals
>>>> * Precision.equals
>>>> * Precision.equalsIncludingNaN
>>>> ?
>>>>
>>> No. its the same as all the three methods with just 2 double
>>> attributes and a boolean return
>>>
>>>>
>>>> Or is the issue about performance?
>>>> If so the problem must be identified and the corresponding method
>>>> improved.
>>>>
>>>> Yes it was performance related issue with Precision.equalsIncludingNan
>> and hence will improve that method
>>
>>> [...]
>>>>
>>>
> I've attached a benchmark on the JIRA page.
>
Thanks for testing,but curious to know your benchmark code/test setup etc
and the N parameter
However In my measurements in my machine with respect to the
testMath1130ForDoubleEqual in PrecisionTest.java attached
i j d time in seconds(old
code) time in seconds(with venkats code)
10 10 10
00.027 00.003
10 10 100
00.020 00.011
10 10 1000
00.038 00.023
10 100 1000
00.066 00.073
10 1000 1000
00.287 00.127
10 10000 1000
02.280 00.947
10 10000 10000
22.217 09.275
10 10000 100000
224.454 91.918
10 100000 10000
224.673 91.319
It's not obvious that it's worth changing the current code (doing so will
> obviously make it less clear).
>
> Well trying to understand this a bit more. So as i am seeing most times
the above timings...
Wondering where am i seeing the gap as at every level (i,j,d) there is a
clear difference from (10s of milliseconds to 100s of seconds)(bigger as
the iterations go up)
Basically If only one of them is NaN it does not make sense to get to a
detailed compare which is what i have eliminated.
Iam just following this tradition already existing in say
MathArrays.equals(final float[] x, final float[] y) method where null
checks are eliminated earlier in this fashion.
Also if x!=x seems cryptic, i could replace with Double.isNaN() to make it
bit obvious.(is this is the concern?)
Please help me as to what is less clear
We've had this kind of argument in the past, with diverging opinions...
>
>
>
> Regards,
> Gilles
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
Re: [MATH-1130] Patch on canequal (math-1130-canequal.patch) to
make faster equals check with nan
Posted by Gilles <gi...@harfang.homelinux.org>.
Hi.
>>>> [...]
>>>>
>>>> The change i am bringing in is simple in that in any case of
>>>> either of
>>>> them nan or both then return true or false and if and only if both
>>>> are
>>>> *not nan* then get to Precision.equals
>>>>
>>>> Does any on have a concern adding this method or if the change
>>>> needs
>>>> to be done else where such as in Precision.equalsIncludingNaN
>>>
>>>
>>> What is the semantics of this new method?
>>> Is it different from all of the following:
>>> * MathUtils.equals
>>> * Precision.equals
>>> * Precision.equalsIncludingNaN
>>> ?
>> No. its the same as all the three methods with just 2 double
>> attributes and a boolean return
>>>
>>> Or is the issue about performance?
>>> If so the problem must be identified and the corresponding method
>>> improved.
>>>
> Yes it was performance related issue with
> Precision.equalsIncludingNan
> and hence will improve that method
>>> [...]
I've attached a benchmark on the JIRA page.
It's not obvious that it's worth changing the current code (doing so
will
obviously make it less clear).
We've had this kind of argument in the past, with diverging opinions...
Regards,
Gilles
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [MATH-1130] Patch on canequal (math-1130-canequal.patch) to make
faster equals check with nan
Posted by venkatesha murthy <ve...@gmail.com>.
On Tue, Jun 24, 2014 at 8:36 AM, venkatesha murthy
<ve...@gmail.com> wrote:
> On Tue, Jun 24, 2014 at 1:46 AM, Gilles <gi...@harfang.homelinux.org> wrote:
>> Hi.
>>
>>
>> On Tue, 24 Jun 2014 00:06:16 +0530, venkatesha murthy wrote:
>>>
>>> I am adding a canEqual method in MathUtils since neither Math.equals nor
>>>
>>> Precision.equalsIncludingNaN performs better under heavy load (I have
>>> attached the test code for the same in the patch).
>>>
>>> The reason as far as i can think is :
>>> In case of equals it keeps churning new objects and makes Double.equals
>>> and in case of Precision.equalsIncludingNaN it might be that
>>> it has not covered the case of quick elimination of detecting one of
>>> them as nan as in that case it can return false.
>>>
>>>
>>> The change i am bringing in is simple in that in any case of either of
>>> them nan or both then return true or false and if and only if both are
>>> *not nan* then get to Precision.equals
>>>
>>> Does any on have a concern adding this method or if the change needs
>>> to be done else where such as in Precision.equalsIncludingNaN
>>
>>
>> What is the semantics of this new method?
>> Is it different from all of the following:
>> * MathUtils.equals
>> * Precision.equals
>> * Precision.equalsIncludingNaN
>> ?
> No. its the same as all the three methods with just 2 double
> attributes and a boolean return
>>
>> Or is the issue about performance?
>> If so the problem must be identified and the corresponding method improved.
>>
Yes it was performance related issue with Precision.equalsIncludingNan
and hence will improve that method
> In that case i am planning to improve Precision.equalsIncludingNaN
> method(s). I will send the patch for the same (after removing
> canEqual) along with modified patch on methods that are using
> canEqual. Hope this is fine.
>>
>> Best regards,
>> Gilles
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [MATH-1130] Patch on canequal (math-1130-canequal.patch) to make
faster equals check with nan
Posted by venkatesha murthy <ve...@gmail.com>.
On Tue, Jun 24, 2014 at 1:46 AM, Gilles <gi...@harfang.homelinux.org> wrote:
> Hi.
>
>
> On Tue, 24 Jun 2014 00:06:16 +0530, venkatesha murthy wrote:
>>
>> I am adding a canEqual method in MathUtils since neither Math.equals nor
>>
>> Precision.equalsIncludingNaN performs better under heavy load (I have
>> attached the test code for the same in the patch).
>>
>> The reason as far as i can think is :
>> In case of equals it keeps churning new objects and makes Double.equals
>> and in case of Precision.equalsIncludingNaN it might be that
>> it has not covered the case of quick elimination of detecting one of
>> them as nan as in that case it can return false.
>>
>>
>> The change i am bringing in is simple in that in any case of either of
>> them nan or both then return true or false and if and only if both are
>> *not nan* then get to Precision.equals
>>
>> Does any on have a concern adding this method or if the change needs
>> to be done else where such as in Precision.equalsIncludingNaN
>
>
> What is the semantics of this new method?
> Is it different from all of the following:
> * MathUtils.equals
> * Precision.equals
> * Precision.equalsIncludingNaN
> ?
No. its the same as all the three methods with just 2 double
attributes and a boolean return
>
> Or is the issue about performance?
> If so the problem must be identified and the corresponding method improved.
>
In that case i am planning to improve Precision.equalsIncludingNaN
method(s). I will send the patch for the same (after removing
canEqual) along with modified patch on methods that are using
canEqual. Hope this is fine.
>
> Best regards,
> Gilles
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [MATH-1130] Patch on canequal (math-1130-canequal.patch) to
make faster equals check with nan
Posted by Gilles <gi...@harfang.homelinux.org>.
Hi.
On Tue, 24 Jun 2014 00:06:16 +0530, venkatesha murthy wrote:
> I am adding a canEqual method in MathUtils since neither Math.equals
> nor
>
> Precision.equalsIncludingNaN performs better under heavy load (I have
> attached the test code for the same in the patch).
>
> The reason as far as i can think is :
> In case of equals it keeps churning new objects and makes
> Double.equals
> and in case of Precision.equalsIncludingNaN it might be that
> it has not covered the case of quick elimination of detecting one of
> them as nan as in that case it can return false.
>
>
> The change i am bringing in is simple in that in any case of either
> of
> them nan or both then return true or false and if and only if both
> are
> *not nan* then get to Precision.equals
>
> Does any on have a concern adding this method or if the change needs
> to be done else where such as in Precision.equalsIncludingNaN
What is the semantics of this new method?
Is it different from all of the following:
* MathUtils.equals
* Precision.equals
* Precision.equalsIncludingNaN
?
Or is the issue about performance?
If so the problem must be identified and the corresponding method
improved.
Best regards,
Gilles
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org