You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Thomas Neidhart <th...@gmail.com> on 2015/01/21 11:43:52 UTC
Re: [math] Add temporary check for rare test failure.
Hi,
I have re-run the jenkins build for commons-math after this change several
times, also on H10 and it seems the test failures have disappeared.
Any objection to keep this?
Thomas
On Wed, Jan 21, 2015 at 12:42 AM, <tn...@apache.org> wrote:
> Repository: commons-math
> Updated Branches:
> refs/heads/master 4e1958256 -> 15bdcc3be
>
>
> Add temporary check for rare test failure.
>
>
> Project: http://git-wip-us.apache.org/repos/asf/commons-math/repo
> Commit:
> http://git-wip-us.apache.org/repos/asf/commons-math/commit/15bdcc3b
> Tree: http://git-wip-us.apache.org/repos/asf/commons-math/tree/15bdcc3b
> Diff: http://git-wip-us.apache.org/repos/asf/commons-math/diff/15bdcc3b
>
> Branch: refs/heads/master
> Commit: 15bdcc3be2b84f68ddcd822da52ef045ed89e57b
> Parents: 4e19582
> Author: Thomas Neidhart <th...@gmail.com>
> Authored: Wed Jan 21 00:42:16 2015 +0100
> Committer: Thomas Neidhart <th...@gmail.com>
> Committed: Wed Jan 21 00:42:16 2015 +0100
>
> ----------------------------------------------------------------------
> src/main/java/org/apache/commons/math3/util/FastMath.java | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
> ----------------------------------------------------------------------
>
>
>
> http://git-wip-us.apache.org/repos/asf/commons-math/blob/15bdcc3b/src/main/java/org/apache/commons/math3/util/FastMath.java
> ----------------------------------------------------------------------
> diff --git a/src/main/java/org/apache/commons/math3/util/FastMath.java
> b/src/main/java/org/apache/commons/math3/util/FastMath.java
> index 5178215..09c5976 100644
> --- a/src/main/java/org/apache/commons/math3/util/FastMath.java
> +++ b/src/main/java/org/apache/commons/math3/util/FastMath.java
> @@ -876,7 +876,11 @@ public class FastMath {
> if (x < 0.0) {
> intVal = (int) -x;
>
> - if (intVal > 746) {
> + // TEMP: special handling of negative_infinity
> + // the above might fail in non-reproducible ways with Sun JDK
> 1.5,
> + // most likely due to a bug in the JIT. Add a safe-guard for
> very
> + // negative numbers.
> + if (intVal > 746 || x < Integer.MIN_VALUE) {
> if (hiPrec != null) {
> hiPrec[0] = 0.0;
> hiPrec[1] = 0.0;
>
>
Re: [math] Add temporary check for rare test failure.
Posted by sebb <se...@gmail.com>.
On 22 January 2015 at 11:44, sebb <se...@gmail.com> wrote:
> On 22 January 2015 at 01:59, Phil Steitz <ph...@gmail.com> wrote:
>> On 1/21/15 4:56 PM, sebb wrote:
>>> On 21 January 2015 at 20:39, Thomas Neidhart <th...@gmail.com> wrote:
>>>> On 01/21/2015 05:32 PM, Phil Steitz wrote:
>>>>> On 1/21/15 3:43 AM, Thomas Neidhart wrote:
>>>>>> Hi,
>>>>>>
>>>>>> I have re-run the jenkins build for commons-math after this change several
>>>>>> times, also on H10 and it seems the test failures have disappeared.
>>>>>>
>>>>>> Any objection to keep this?
>>>>> -0
>>>>>
>>>>> Is there a way to selectively suppress tests depending on JDK?
>>>> there are many different tests that failed.
>>>> We could either add a profile for java 1.5 to suppress them, or
>>>> something similar as in collections, were individual tests are skipped
>>>> when executed with a certain JVM due to known bugs.
>>> What effect does the change have on systems that don't have the JIT bug?
>>
>> Have a look at the diff below. Probably trivial, but non-zero per
>> impact, as it adds one compare for some extreme arguments.
>
> Actually, what I should have written was: can the change affect the
> return value?
>
> Note: if it is possible to reliably (and cheaply) detect the likely
> presence of the bug, it might be possible to create a static final
> boolean which controls whether the extra test is needed or not.
> The JIT should optimise away any code that is conditional upon a
> boolean that is always false.
>
> Though looking at the code again, I wonder if intval needs to be
> forced positive.
> If not, this would avoid the need to check against Integer.MIN_VALUE.
>
> I did a quick check locally, and it seems to work OK for me on MacOSx.
>
> Obviously one has to change (intVal > 746) to (intVal < -746) etc. but
> intVal is not used much so the changes are minor
>
> When this is done, the setup of intPartA and intPartB is the same for
> both and can be done after the check for (x < 0.0).
>
> Though I would suggest the check should be changed to (intVal < 0)
> instead, as that is what now matters to the code in the conditional.
>
> Does that make sense or have I overlooked something?
Looks like I have: cannot replace (x < 0.0) with (intVal < 0), as that
causes test failures.
This is because intVal can be 0 for x < 0.0.
>>>
>>> I tend to agree that fixing the code to avoid a bug in a single JVM is
>>> not ideal.
>>>
>>> Especially given that the bug only seems to happen on Java 5, and CM
>>> is likely to move away from that for the next release anyway.
>>
>> We will probably cut some more 3.x releases, so will want to be able
>> to test with 1.5. So probably Thoma' suggestion above is best.
>
> OK
>
>> Phil
>>>
>>>> Thomas
>>>>
>>>>> Phil
>>>>>> Thomas
>>>>>>
>>>>>> On Wed, Jan 21, 2015 at 12:42 AM, <tn...@apache.org> wrote:
>>>>>>
>>>>>>> Repository: commons-math
>>>>>>> Updated Branches:
>>>>>>> refs/heads/master 4e1958256 -> 15bdcc3be
>>>>>>>
>>>>>>>
>>>>>>> Add temporary check for rare test failure.
>>>>>>>
>>>>>>>
>>>>>>> Project: http://git-wip-us.apache.org/repos/asf/commons-math/repo
>>>>>>> Commit:
>>>>>>> http://git-wip-us.apache.org/repos/asf/commons-math/commit/15bdcc3b
>>>>>>> Tree: http://git-wip-us.apache.org/repos/asf/commons-math/tree/15bdcc3b
>>>>>>> Diff: http://git-wip-us.apache.org/repos/asf/commons-math/diff/15bdcc3b
>>>>>>>
>>>>>>> Branch: refs/heads/master
>>>>>>> Commit: 15bdcc3be2b84f68ddcd822da52ef045ed89e57b
>>>>>>> Parents: 4e19582
>>>>>>> Author: Thomas Neidhart <th...@gmail.com>
>>>>>>> Authored: Wed Jan 21 00:42:16 2015 +0100
>>>>>>> Committer: Thomas Neidhart <th...@gmail.com>
>>>>>>> Committed: Wed Jan 21 00:42:16 2015 +0100
>>>>>>>
>>>>>>> ----------------------------------------------------------------------
>>>>>>> src/main/java/org/apache/commons/math3/util/FastMath.java | 6 +++++-
>>>>>>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>>>>>> ----------------------------------------------------------------------
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> http://git-wip-us.apache.org/repos/asf/commons-math/blob/15bdcc3b/src/main/java/org/apache/commons/math3/util/FastMath.java
>>>>>>> ----------------------------------------------------------------------
>>>>>>> diff --git a/src/main/java/org/apache/commons/math3/util/FastMath.java
>>>>>>> b/src/main/java/org/apache/commons/math3/util/FastMath.java
>>>>>>> index 5178215..09c5976 100644
>>>>>>> --- a/src/main/java/org/apache/commons/math3/util/FastMath.java
>>>>>>> +++ b/src/main/java/org/apache/commons/math3/util/FastMath.java
>>>>>>> @@ -876,7 +876,11 @@ public class FastMath {
>>>>>>> if (x < 0.0) {
>>>>>>> intVal = (int) -x;
>>>>>>>
>>>>>>> - if (intVal > 746) {
>>>>>>> + // TEMP: special handling of negative_infinity
>>>>>>> + // the above might fail in non-reproducible ways with Sun JDK
>>>>>>> 1.5,
>>>>>>> + // most likely due to a bug in the JIT. Add a safe-guard for
>>>>>>> very
>>>>>>> + // negative numbers.
>>>>>>> + if (intVal > 746 || x < Integer.MIN_VALUE) {
>>>>>>> if (hiPrec != null) {
>>>>>>> hiPrec[0] = 0.0;
>>>>>>> hiPrec[1] = 0.0;
>>>>>>>
>>>>>>>
>>>>>
>>>>> ---------------------------------------------------------------------
>>>>> 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
>>>>
>>> ---------------------------------------------------------------------
>>> 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
>>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [math] Add temporary check for rare test failure.
Posted by sebb <se...@gmail.com>.
On 22 January 2015 at 12:22, Thomas Neidhart <th...@gmail.com> wrote:
> What about this change:
>
> if (x < 0.0) {
> if (x < -746d) {
> if (hiPrec != null) {
> hiPrec[0] = 0.0;
> hiPrec[1] = 0.0;
> }
> return 0.0;
> }
>
> intVal = (int) -x;
> if (intVal > 709) {
>
> So the first comparison we do using the double value to filter out extreme
> negative values, the rest is unchanged.
That would work, but the code is not as efficient as it could be,
because intVal has to be converted back to negative at the end.
> Thomas
>
>
> On Thu, Jan 22, 2015 at 12:44 PM, sebb <se...@gmail.com> wrote:
>
>> On 22 January 2015 at 01:59, Phil Steitz <ph...@gmail.com> wrote:
>> > On 1/21/15 4:56 PM, sebb wrote:
>> >> On 21 January 2015 at 20:39, Thomas Neidhart <th...@gmail.com>
>> wrote:
>> >>> On 01/21/2015 05:32 PM, Phil Steitz wrote:
>> >>>> On 1/21/15 3:43 AM, Thomas Neidhart wrote:
>> >>>>> Hi,
>> >>>>>
>> >>>>> I have re-run the jenkins build for commons-math after this change
>> several
>> >>>>> times, also on H10 and it seems the test failures have disappeared.
>> >>>>>
>> >>>>> Any objection to keep this?
>> >>>> -0
>> >>>>
>> >>>> Is there a way to selectively suppress tests depending on JDK?
>> >>> there are many different tests that failed.
>> >>> We could either add a profile for java 1.5 to suppress them, or
>> >>> something similar as in collections, were individual tests are skipped
>> >>> when executed with a certain JVM due to known bugs.
>> >> What effect does the change have on systems that don't have the JIT bug?
>> >
>> > Have a look at the diff below. Probably trivial, but non-zero per
>> > impact, as it adds one compare for some extreme arguments.
>>
>> Actually, what I should have written was: can the change affect the
>> return value?
>>
>> Note: if it is possible to reliably (and cheaply) detect the likely
>> presence of the bug, it might be possible to create a static final
>> boolean which controls whether the extra test is needed or not.
>> The JIT should optimise away any code that is conditional upon a
>> boolean that is always false.
>>
>> Though looking at the code again, I wonder if intval needs to be
>> forced positive.
>> If not, this would avoid the need to check against Integer.MIN_VALUE.
>>
>> I did a quick check locally, and it seems to work OK for me on MacOSx.
>>
>> Obviously one has to change (intVal > 746) to (intVal < -746) etc. but
>> intVal is not used much so the changes are minor
>>
>> When this is done, the setup of intPartA and intPartB is the same for
>> both and can be done after the check for (x < 0.0).
>>
>> Though I would suggest the check should be changed to (intVal < 0)
>> instead, as that is what now matters to the code in the conditional.
>>
>> Does that make sense or have I overlooked something?
>>
>> >>
>> >> I tend to agree that fixing the code to avoid a bug in a single JVM is
>> >> not ideal.
>> >>
>> >> Especially given that the bug only seems to happen on Java 5, and CM
>> >> is likely to move away from that for the next release anyway.
>> >
>> > We will probably cut some more 3.x releases, so will want to be able
>> > to test with 1.5. So probably Thoma' suggestion above is best.
>>
>> OK
>>
>> > Phil
>> >>
>> >>> Thomas
>> >>>
>> >>>> Phil
>> >>>>> Thomas
>> >>>>>
>> >>>>> On Wed, Jan 21, 2015 at 12:42 AM, <tn...@apache.org> wrote:
>> >>>>>
>> >>>>>> Repository: commons-math
>> >>>>>> Updated Branches:
>> >>>>>> refs/heads/master 4e1958256 -> 15bdcc3be
>> >>>>>>
>> >>>>>>
>> >>>>>> Add temporary check for rare test failure.
>> >>>>>>
>> >>>>>>
>> >>>>>> Project: http://git-wip-us.apache.org/repos/asf/commons-math/repo
>> >>>>>> Commit:
>> >>>>>> http://git-wip-us.apache.org/repos/asf/commons-math/commit/15bdcc3b
>> >>>>>> Tree:
>> http://git-wip-us.apache.org/repos/asf/commons-math/tree/15bdcc3b
>> >>>>>> Diff:
>> http://git-wip-us.apache.org/repos/asf/commons-math/diff/15bdcc3b
>> >>>>>>
>> >>>>>> Branch: refs/heads/master
>> >>>>>> Commit: 15bdcc3be2b84f68ddcd822da52ef045ed89e57b
>> >>>>>> Parents: 4e19582
>> >>>>>> Author: Thomas Neidhart <th...@gmail.com>
>> >>>>>> Authored: Wed Jan 21 00:42:16 2015 +0100
>> >>>>>> Committer: Thomas Neidhart <th...@gmail.com>
>> >>>>>> Committed: Wed Jan 21 00:42:16 2015 +0100
>> >>>>>>
>> >>>>>>
>> ----------------------------------------------------------------------
>> >>>>>> src/main/java/org/apache/commons/math3/util/FastMath.java | 6
>> +++++-
>> >>>>>> 1 file changed, 5 insertions(+), 1 deletion(-)
>> >>>>>>
>> ----------------------------------------------------------------------
>> >>>>>>
>> >>>>>>
>> >>>>>>
>> >>>>>>
>> http://git-wip-us.apache.org/repos/asf/commons-math/blob/15bdcc3b/src/main/java/org/apache/commons/math3/util/FastMath.java
>> >>>>>>
>> ----------------------------------------------------------------------
>> >>>>>> diff --git
>> a/src/main/java/org/apache/commons/math3/util/FastMath.java
>> >>>>>> b/src/main/java/org/apache/commons/math3/util/FastMath.java
>> >>>>>> index 5178215..09c5976 100644
>> >>>>>> --- a/src/main/java/org/apache/commons/math3/util/FastMath.java
>> >>>>>> +++ b/src/main/java/org/apache/commons/math3/util/FastMath.java
>> >>>>>> @@ -876,7 +876,11 @@ public class FastMath {
>> >>>>>> if (x < 0.0) {
>> >>>>>> intVal = (int) -x;
>> >>>>>>
>> >>>>>> - if (intVal > 746) {
>> >>>>>> + // TEMP: special handling of negative_infinity
>> >>>>>> + // the above might fail in non-reproducible ways with
>> Sun JDK
>> >>>>>> 1.5,
>> >>>>>> + // most likely due to a bug in the JIT. Add a
>> safe-guard for
>> >>>>>> very
>> >>>>>> + // negative numbers.
>> >>>>>> + if (intVal > 746 || x < Integer.MIN_VALUE) {
>> >>>>>> if (hiPrec != null) {
>> >>>>>> hiPrec[0] = 0.0;
>> >>>>>> hiPrec[1] = 0.0;
>> >>>>>>
>> >>>>>>
>> >>>>
>> >>>> ---------------------------------------------------------------------
>> >>>> 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
>> >>>
>> >> ---------------------------------------------------------------------
>> >> 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
>> >
>>
>> ---------------------------------------------------------------------
>> 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] Add temporary check for rare test failure.
Posted by Thomas Neidhart <th...@gmail.com>.
What about this change:
if (x < 0.0) {
if (x < -746d) {
if (hiPrec != null) {
hiPrec[0] = 0.0;
hiPrec[1] = 0.0;
}
return 0.0;
}
intVal = (int) -x;
if (intVal > 709) {
So the first comparison we do using the double value to filter out extreme
negative values, the rest is unchanged.
Thomas
On Thu, Jan 22, 2015 at 12:44 PM, sebb <se...@gmail.com> wrote:
> On 22 January 2015 at 01:59, Phil Steitz <ph...@gmail.com> wrote:
> > On 1/21/15 4:56 PM, sebb wrote:
> >> On 21 January 2015 at 20:39, Thomas Neidhart <th...@gmail.com>
> wrote:
> >>> On 01/21/2015 05:32 PM, Phil Steitz wrote:
> >>>> On 1/21/15 3:43 AM, Thomas Neidhart wrote:
> >>>>> Hi,
> >>>>>
> >>>>> I have re-run the jenkins build for commons-math after this change
> several
> >>>>> times, also on H10 and it seems the test failures have disappeared.
> >>>>>
> >>>>> Any objection to keep this?
> >>>> -0
> >>>>
> >>>> Is there a way to selectively suppress tests depending on JDK?
> >>> there are many different tests that failed.
> >>> We could either add a profile for java 1.5 to suppress them, or
> >>> something similar as in collections, were individual tests are skipped
> >>> when executed with a certain JVM due to known bugs.
> >> What effect does the change have on systems that don't have the JIT bug?
> >
> > Have a look at the diff below. Probably trivial, but non-zero per
> > impact, as it adds one compare for some extreme arguments.
>
> Actually, what I should have written was: can the change affect the
> return value?
>
> Note: if it is possible to reliably (and cheaply) detect the likely
> presence of the bug, it might be possible to create a static final
> boolean which controls whether the extra test is needed or not.
> The JIT should optimise away any code that is conditional upon a
> boolean that is always false.
>
> Though looking at the code again, I wonder if intval needs to be
> forced positive.
> If not, this would avoid the need to check against Integer.MIN_VALUE.
>
> I did a quick check locally, and it seems to work OK for me on MacOSx.
>
> Obviously one has to change (intVal > 746) to (intVal < -746) etc. but
> intVal is not used much so the changes are minor
>
> When this is done, the setup of intPartA and intPartB is the same for
> both and can be done after the check for (x < 0.0).
>
> Though I would suggest the check should be changed to (intVal < 0)
> instead, as that is what now matters to the code in the conditional.
>
> Does that make sense or have I overlooked something?
>
> >>
> >> I tend to agree that fixing the code to avoid a bug in a single JVM is
> >> not ideal.
> >>
> >> Especially given that the bug only seems to happen on Java 5, and CM
> >> is likely to move away from that for the next release anyway.
> >
> > We will probably cut some more 3.x releases, so will want to be able
> > to test with 1.5. So probably Thoma' suggestion above is best.
>
> OK
>
> > Phil
> >>
> >>> Thomas
> >>>
> >>>> Phil
> >>>>> Thomas
> >>>>>
> >>>>> On Wed, Jan 21, 2015 at 12:42 AM, <tn...@apache.org> wrote:
> >>>>>
> >>>>>> Repository: commons-math
> >>>>>> Updated Branches:
> >>>>>> refs/heads/master 4e1958256 -> 15bdcc3be
> >>>>>>
> >>>>>>
> >>>>>> Add temporary check for rare test failure.
> >>>>>>
> >>>>>>
> >>>>>> Project: http://git-wip-us.apache.org/repos/asf/commons-math/repo
> >>>>>> Commit:
> >>>>>> http://git-wip-us.apache.org/repos/asf/commons-math/commit/15bdcc3b
> >>>>>> Tree:
> http://git-wip-us.apache.org/repos/asf/commons-math/tree/15bdcc3b
> >>>>>> Diff:
> http://git-wip-us.apache.org/repos/asf/commons-math/diff/15bdcc3b
> >>>>>>
> >>>>>> Branch: refs/heads/master
> >>>>>> Commit: 15bdcc3be2b84f68ddcd822da52ef045ed89e57b
> >>>>>> Parents: 4e19582
> >>>>>> Author: Thomas Neidhart <th...@gmail.com>
> >>>>>> Authored: Wed Jan 21 00:42:16 2015 +0100
> >>>>>> Committer: Thomas Neidhart <th...@gmail.com>
> >>>>>> Committed: Wed Jan 21 00:42:16 2015 +0100
> >>>>>>
> >>>>>>
> ----------------------------------------------------------------------
> >>>>>> src/main/java/org/apache/commons/math3/util/FastMath.java | 6
> +++++-
> >>>>>> 1 file changed, 5 insertions(+), 1 deletion(-)
> >>>>>>
> ----------------------------------------------------------------------
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> http://git-wip-us.apache.org/repos/asf/commons-math/blob/15bdcc3b/src/main/java/org/apache/commons/math3/util/FastMath.java
> >>>>>>
> ----------------------------------------------------------------------
> >>>>>> diff --git
> a/src/main/java/org/apache/commons/math3/util/FastMath.java
> >>>>>> b/src/main/java/org/apache/commons/math3/util/FastMath.java
> >>>>>> index 5178215..09c5976 100644
> >>>>>> --- a/src/main/java/org/apache/commons/math3/util/FastMath.java
> >>>>>> +++ b/src/main/java/org/apache/commons/math3/util/FastMath.java
> >>>>>> @@ -876,7 +876,11 @@ public class FastMath {
> >>>>>> if (x < 0.0) {
> >>>>>> intVal = (int) -x;
> >>>>>>
> >>>>>> - if (intVal > 746) {
> >>>>>> + // TEMP: special handling of negative_infinity
> >>>>>> + // the above might fail in non-reproducible ways with
> Sun JDK
> >>>>>> 1.5,
> >>>>>> + // most likely due to a bug in the JIT. Add a
> safe-guard for
> >>>>>> very
> >>>>>> + // negative numbers.
> >>>>>> + if (intVal > 746 || x < Integer.MIN_VALUE) {
> >>>>>> if (hiPrec != null) {
> >>>>>> hiPrec[0] = 0.0;
> >>>>>> hiPrec[1] = 0.0;
> >>>>>>
> >>>>>>
> >>>>
> >>>> ---------------------------------------------------------------------
> >>>> 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
> >>>
> >> ---------------------------------------------------------------------
> >> 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
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
Re: [math] Add temporary check for rare test failure.
Posted by sebb <se...@gmail.com>.
On 22 January 2015 at 01:59, Phil Steitz <ph...@gmail.com> wrote:
> On 1/21/15 4:56 PM, sebb wrote:
>> On 21 January 2015 at 20:39, Thomas Neidhart <th...@gmail.com> wrote:
>>> On 01/21/2015 05:32 PM, Phil Steitz wrote:
>>>> On 1/21/15 3:43 AM, Thomas Neidhart wrote:
>>>>> Hi,
>>>>>
>>>>> I have re-run the jenkins build for commons-math after this change several
>>>>> times, also on H10 and it seems the test failures have disappeared.
>>>>>
>>>>> Any objection to keep this?
>>>> -0
>>>>
>>>> Is there a way to selectively suppress tests depending on JDK?
>>> there are many different tests that failed.
>>> We could either add a profile for java 1.5 to suppress them, or
>>> something similar as in collections, were individual tests are skipped
>>> when executed with a certain JVM due to known bugs.
>> What effect does the change have on systems that don't have the JIT bug?
>
> Have a look at the diff below. Probably trivial, but non-zero per
> impact, as it adds one compare for some extreme arguments.
Actually, what I should have written was: can the change affect the
return value?
Note: if it is possible to reliably (and cheaply) detect the likely
presence of the bug, it might be possible to create a static final
boolean which controls whether the extra test is needed or not.
The JIT should optimise away any code that is conditional upon a
boolean that is always false.
Though looking at the code again, I wonder if intval needs to be
forced positive.
If not, this would avoid the need to check against Integer.MIN_VALUE.
I did a quick check locally, and it seems to work OK for me on MacOSx.
Obviously one has to change (intVal > 746) to (intVal < -746) etc. but
intVal is not used much so the changes are minor
When this is done, the setup of intPartA and intPartB is the same for
both and can be done after the check for (x < 0.0).
Though I would suggest the check should be changed to (intVal < 0)
instead, as that is what now matters to the code in the conditional.
Does that make sense or have I overlooked something?
>>
>> I tend to agree that fixing the code to avoid a bug in a single JVM is
>> not ideal.
>>
>> Especially given that the bug only seems to happen on Java 5, and CM
>> is likely to move away from that for the next release anyway.
>
> We will probably cut some more 3.x releases, so will want to be able
> to test with 1.5. So probably Thoma' suggestion above is best.
OK
> Phil
>>
>>> Thomas
>>>
>>>> Phil
>>>>> Thomas
>>>>>
>>>>> On Wed, Jan 21, 2015 at 12:42 AM, <tn...@apache.org> wrote:
>>>>>
>>>>>> Repository: commons-math
>>>>>> Updated Branches:
>>>>>> refs/heads/master 4e1958256 -> 15bdcc3be
>>>>>>
>>>>>>
>>>>>> Add temporary check for rare test failure.
>>>>>>
>>>>>>
>>>>>> Project: http://git-wip-us.apache.org/repos/asf/commons-math/repo
>>>>>> Commit:
>>>>>> http://git-wip-us.apache.org/repos/asf/commons-math/commit/15bdcc3b
>>>>>> Tree: http://git-wip-us.apache.org/repos/asf/commons-math/tree/15bdcc3b
>>>>>> Diff: http://git-wip-us.apache.org/repos/asf/commons-math/diff/15bdcc3b
>>>>>>
>>>>>> Branch: refs/heads/master
>>>>>> Commit: 15bdcc3be2b84f68ddcd822da52ef045ed89e57b
>>>>>> Parents: 4e19582
>>>>>> Author: Thomas Neidhart <th...@gmail.com>
>>>>>> Authored: Wed Jan 21 00:42:16 2015 +0100
>>>>>> Committer: Thomas Neidhart <th...@gmail.com>
>>>>>> Committed: Wed Jan 21 00:42:16 2015 +0100
>>>>>>
>>>>>> ----------------------------------------------------------------------
>>>>>> src/main/java/org/apache/commons/math3/util/FastMath.java | 6 +++++-
>>>>>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>>>>> ----------------------------------------------------------------------
>>>>>>
>>>>>>
>>>>>>
>>>>>> http://git-wip-us.apache.org/repos/asf/commons-math/blob/15bdcc3b/src/main/java/org/apache/commons/math3/util/FastMath.java
>>>>>> ----------------------------------------------------------------------
>>>>>> diff --git a/src/main/java/org/apache/commons/math3/util/FastMath.java
>>>>>> b/src/main/java/org/apache/commons/math3/util/FastMath.java
>>>>>> index 5178215..09c5976 100644
>>>>>> --- a/src/main/java/org/apache/commons/math3/util/FastMath.java
>>>>>> +++ b/src/main/java/org/apache/commons/math3/util/FastMath.java
>>>>>> @@ -876,7 +876,11 @@ public class FastMath {
>>>>>> if (x < 0.0) {
>>>>>> intVal = (int) -x;
>>>>>>
>>>>>> - if (intVal > 746) {
>>>>>> + // TEMP: special handling of negative_infinity
>>>>>> + // the above might fail in non-reproducible ways with Sun JDK
>>>>>> 1.5,
>>>>>> + // most likely due to a bug in the JIT. Add a safe-guard for
>>>>>> very
>>>>>> + // negative numbers.
>>>>>> + if (intVal > 746 || x < Integer.MIN_VALUE) {
>>>>>> if (hiPrec != null) {
>>>>>> hiPrec[0] = 0.0;
>>>>>> hiPrec[1] = 0.0;
>>>>>>
>>>>>>
>>>>
>>>> ---------------------------------------------------------------------
>>>> 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
>>>
>> ---------------------------------------------------------------------
>> 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
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [math] Add temporary check for rare test failure.
Posted by Phil Steitz <ph...@gmail.com>.
On 1/21/15 4:56 PM, sebb wrote:
> On 21 January 2015 at 20:39, Thomas Neidhart <th...@gmail.com> wrote:
>> On 01/21/2015 05:32 PM, Phil Steitz wrote:
>>> On 1/21/15 3:43 AM, Thomas Neidhart wrote:
>>>> Hi,
>>>>
>>>> I have re-run the jenkins build for commons-math after this change several
>>>> times, also on H10 and it seems the test failures have disappeared.
>>>>
>>>> Any objection to keep this?
>>> -0
>>>
>>> Is there a way to selectively suppress tests depending on JDK?
>> there are many different tests that failed.
>> We could either add a profile for java 1.5 to suppress them, or
>> something similar as in collections, were individual tests are skipped
>> when executed with a certain JVM due to known bugs.
> What effect does the change have on systems that don't have the JIT bug?
Have a look at the diff below. Probably trivial, but non-zero per
impact, as it adds one compare for some extreme arguments.
>
> I tend to agree that fixing the code to avoid a bug in a single JVM is
> not ideal.
>
> Especially given that the bug only seems to happen on Java 5, and CM
> is likely to move away from that for the next release anyway.
We will probably cut some more 3.x releases, so will want to be able
to test with 1.5. So probably Thoma' suggestion above is best.
Phil
>
>> Thomas
>>
>>> Phil
>>>> Thomas
>>>>
>>>> On Wed, Jan 21, 2015 at 12:42 AM, <tn...@apache.org> wrote:
>>>>
>>>>> Repository: commons-math
>>>>> Updated Branches:
>>>>> refs/heads/master 4e1958256 -> 15bdcc3be
>>>>>
>>>>>
>>>>> Add temporary check for rare test failure.
>>>>>
>>>>>
>>>>> Project: http://git-wip-us.apache.org/repos/asf/commons-math/repo
>>>>> Commit:
>>>>> http://git-wip-us.apache.org/repos/asf/commons-math/commit/15bdcc3b
>>>>> Tree: http://git-wip-us.apache.org/repos/asf/commons-math/tree/15bdcc3b
>>>>> Diff: http://git-wip-us.apache.org/repos/asf/commons-math/diff/15bdcc3b
>>>>>
>>>>> Branch: refs/heads/master
>>>>> Commit: 15bdcc3be2b84f68ddcd822da52ef045ed89e57b
>>>>> Parents: 4e19582
>>>>> Author: Thomas Neidhart <th...@gmail.com>
>>>>> Authored: Wed Jan 21 00:42:16 2015 +0100
>>>>> Committer: Thomas Neidhart <th...@gmail.com>
>>>>> Committed: Wed Jan 21 00:42:16 2015 +0100
>>>>>
>>>>> ----------------------------------------------------------------------
>>>>> src/main/java/org/apache/commons/math3/util/FastMath.java | 6 +++++-
>>>>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>>>> ----------------------------------------------------------------------
>>>>>
>>>>>
>>>>>
>>>>> http://git-wip-us.apache.org/repos/asf/commons-math/blob/15bdcc3b/src/main/java/org/apache/commons/math3/util/FastMath.java
>>>>> ----------------------------------------------------------------------
>>>>> diff --git a/src/main/java/org/apache/commons/math3/util/FastMath.java
>>>>> b/src/main/java/org/apache/commons/math3/util/FastMath.java
>>>>> index 5178215..09c5976 100644
>>>>> --- a/src/main/java/org/apache/commons/math3/util/FastMath.java
>>>>> +++ b/src/main/java/org/apache/commons/math3/util/FastMath.java
>>>>> @@ -876,7 +876,11 @@ public class FastMath {
>>>>> if (x < 0.0) {
>>>>> intVal = (int) -x;
>>>>>
>>>>> - if (intVal > 746) {
>>>>> + // TEMP: special handling of negative_infinity
>>>>> + // the above might fail in non-reproducible ways with Sun JDK
>>>>> 1.5,
>>>>> + // most likely due to a bug in the JIT. Add a safe-guard for
>>>>> very
>>>>> + // negative numbers.
>>>>> + if (intVal > 746 || x < Integer.MIN_VALUE) {
>>>>> if (hiPrec != null) {
>>>>> hiPrec[0] = 0.0;
>>>>> hiPrec[1] = 0.0;
>>>>>
>>>>>
>>>
>>> ---------------------------------------------------------------------
>>> 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
>>
> ---------------------------------------------------------------------
> 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] Add temporary check for rare test failure.
Posted by sebb <se...@gmail.com>.
On 21 January 2015 at 20:39, Thomas Neidhart <th...@gmail.com> wrote:
> On 01/21/2015 05:32 PM, Phil Steitz wrote:
>> On 1/21/15 3:43 AM, Thomas Neidhart wrote:
>>> Hi,
>>>
>>> I have re-run the jenkins build for commons-math after this change several
>>> times, also on H10 and it seems the test failures have disappeared.
>>>
>>> Any objection to keep this?
>>
>> -0
>>
>> Is there a way to selectively suppress tests depending on JDK?
>
> there are many different tests that failed.
> We could either add a profile for java 1.5 to suppress them, or
> something similar as in collections, were individual tests are skipped
> when executed with a certain JVM due to known bugs.
What effect does the change have on systems that don't have the JIT bug?
I tend to agree that fixing the code to avoid a bug in a single JVM is
not ideal.
Especially given that the bug only seems to happen on Java 5, and CM
is likely to move away from that for the next release anyway.
> Thomas
>
>> Phil
>>>
>>> Thomas
>>>
>>> On Wed, Jan 21, 2015 at 12:42 AM, <tn...@apache.org> wrote:
>>>
>>>> Repository: commons-math
>>>> Updated Branches:
>>>> refs/heads/master 4e1958256 -> 15bdcc3be
>>>>
>>>>
>>>> Add temporary check for rare test failure.
>>>>
>>>>
>>>> Project: http://git-wip-us.apache.org/repos/asf/commons-math/repo
>>>> Commit:
>>>> http://git-wip-us.apache.org/repos/asf/commons-math/commit/15bdcc3b
>>>> Tree: http://git-wip-us.apache.org/repos/asf/commons-math/tree/15bdcc3b
>>>> Diff: http://git-wip-us.apache.org/repos/asf/commons-math/diff/15bdcc3b
>>>>
>>>> Branch: refs/heads/master
>>>> Commit: 15bdcc3be2b84f68ddcd822da52ef045ed89e57b
>>>> Parents: 4e19582
>>>> Author: Thomas Neidhart <th...@gmail.com>
>>>> Authored: Wed Jan 21 00:42:16 2015 +0100
>>>> Committer: Thomas Neidhart <th...@gmail.com>
>>>> Committed: Wed Jan 21 00:42:16 2015 +0100
>>>>
>>>> ----------------------------------------------------------------------
>>>> src/main/java/org/apache/commons/math3/util/FastMath.java | 6 +++++-
>>>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>>> ----------------------------------------------------------------------
>>>>
>>>>
>>>>
>>>> http://git-wip-us.apache.org/repos/asf/commons-math/blob/15bdcc3b/src/main/java/org/apache/commons/math3/util/FastMath.java
>>>> ----------------------------------------------------------------------
>>>> diff --git a/src/main/java/org/apache/commons/math3/util/FastMath.java
>>>> b/src/main/java/org/apache/commons/math3/util/FastMath.java
>>>> index 5178215..09c5976 100644
>>>> --- a/src/main/java/org/apache/commons/math3/util/FastMath.java
>>>> +++ b/src/main/java/org/apache/commons/math3/util/FastMath.java
>>>> @@ -876,7 +876,11 @@ public class FastMath {
>>>> if (x < 0.0) {
>>>> intVal = (int) -x;
>>>>
>>>> - if (intVal > 746) {
>>>> + // TEMP: special handling of negative_infinity
>>>> + // the above might fail in non-reproducible ways with Sun JDK
>>>> 1.5,
>>>> + // most likely due to a bug in the JIT. Add a safe-guard for
>>>> very
>>>> + // negative numbers.
>>>> + if (intVal > 746 || x < Integer.MIN_VALUE) {
>>>> if (hiPrec != null) {
>>>> hiPrec[0] = 0.0;
>>>> hiPrec[1] = 0.0;
>>>>
>>>>
>>
>>
>> ---------------------------------------------------------------------
>> 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
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [math] Add temporary check for rare test failure.
Posted by Thomas Neidhart <th...@gmail.com>.
On 01/21/2015 05:32 PM, Phil Steitz wrote:
> On 1/21/15 3:43 AM, Thomas Neidhart wrote:
>> Hi,
>>
>> I have re-run the jenkins build for commons-math after this change several
>> times, also on H10 and it seems the test failures have disappeared.
>>
>> Any objection to keep this?
>
> -0
>
> Is there a way to selectively suppress tests depending on JDK?
there are many different tests that failed.
We could either add a profile for java 1.5 to suppress them, or
something similar as in collections, were individual tests are skipped
when executed with a certain JVM due to known bugs.
Thomas
> Phil
>>
>> Thomas
>>
>> On Wed, Jan 21, 2015 at 12:42 AM, <tn...@apache.org> wrote:
>>
>>> Repository: commons-math
>>> Updated Branches:
>>> refs/heads/master 4e1958256 -> 15bdcc3be
>>>
>>>
>>> Add temporary check for rare test failure.
>>>
>>>
>>> Project: http://git-wip-us.apache.org/repos/asf/commons-math/repo
>>> Commit:
>>> http://git-wip-us.apache.org/repos/asf/commons-math/commit/15bdcc3b
>>> Tree: http://git-wip-us.apache.org/repos/asf/commons-math/tree/15bdcc3b
>>> Diff: http://git-wip-us.apache.org/repos/asf/commons-math/diff/15bdcc3b
>>>
>>> Branch: refs/heads/master
>>> Commit: 15bdcc3be2b84f68ddcd822da52ef045ed89e57b
>>> Parents: 4e19582
>>> Author: Thomas Neidhart <th...@gmail.com>
>>> Authored: Wed Jan 21 00:42:16 2015 +0100
>>> Committer: Thomas Neidhart <th...@gmail.com>
>>> Committed: Wed Jan 21 00:42:16 2015 +0100
>>>
>>> ----------------------------------------------------------------------
>>> src/main/java/org/apache/commons/math3/util/FastMath.java | 6 +++++-
>>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>> ----------------------------------------------------------------------
>>>
>>>
>>>
>>> http://git-wip-us.apache.org/repos/asf/commons-math/blob/15bdcc3b/src/main/java/org/apache/commons/math3/util/FastMath.java
>>> ----------------------------------------------------------------------
>>> diff --git a/src/main/java/org/apache/commons/math3/util/FastMath.java
>>> b/src/main/java/org/apache/commons/math3/util/FastMath.java
>>> index 5178215..09c5976 100644
>>> --- a/src/main/java/org/apache/commons/math3/util/FastMath.java
>>> +++ b/src/main/java/org/apache/commons/math3/util/FastMath.java
>>> @@ -876,7 +876,11 @@ public class FastMath {
>>> if (x < 0.0) {
>>> intVal = (int) -x;
>>>
>>> - if (intVal > 746) {
>>> + // TEMP: special handling of negative_infinity
>>> + // the above might fail in non-reproducible ways with Sun JDK
>>> 1.5,
>>> + // most likely due to a bug in the JIT. Add a safe-guard for
>>> very
>>> + // negative numbers.
>>> + if (intVal > 746 || x < Integer.MIN_VALUE) {
>>> if (hiPrec != null) {
>>> hiPrec[0] = 0.0;
>>> hiPrec[1] = 0.0;
>>>
>>>
>
>
> ---------------------------------------------------------------------
> 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] Add temporary check for rare test failure.
Posted by Phil Steitz <ph...@gmail.com>.
On 1/21/15 3:43 AM, Thomas Neidhart wrote:
> Hi,
>
> I have re-run the jenkins build for commons-math after this change several
> times, also on H10 and it seems the test failures have disappeared.
>
> Any objection to keep this?
-0
Is there a way to selectively suppress tests depending on JDK?
Phil
>
> Thomas
>
> On Wed, Jan 21, 2015 at 12:42 AM, <tn...@apache.org> wrote:
>
>> Repository: commons-math
>> Updated Branches:
>> refs/heads/master 4e1958256 -> 15bdcc3be
>>
>>
>> Add temporary check for rare test failure.
>>
>>
>> Project: http://git-wip-us.apache.org/repos/asf/commons-math/repo
>> Commit:
>> http://git-wip-us.apache.org/repos/asf/commons-math/commit/15bdcc3b
>> Tree: http://git-wip-us.apache.org/repos/asf/commons-math/tree/15bdcc3b
>> Diff: http://git-wip-us.apache.org/repos/asf/commons-math/diff/15bdcc3b
>>
>> Branch: refs/heads/master
>> Commit: 15bdcc3be2b84f68ddcd822da52ef045ed89e57b
>> Parents: 4e19582
>> Author: Thomas Neidhart <th...@gmail.com>
>> Authored: Wed Jan 21 00:42:16 2015 +0100
>> Committer: Thomas Neidhart <th...@gmail.com>
>> Committed: Wed Jan 21 00:42:16 2015 +0100
>>
>> ----------------------------------------------------------------------
>> src/main/java/org/apache/commons/math3/util/FastMath.java | 6 +++++-
>> 1 file changed, 5 insertions(+), 1 deletion(-)
>> ----------------------------------------------------------------------
>>
>>
>>
>> http://git-wip-us.apache.org/repos/asf/commons-math/blob/15bdcc3b/src/main/java/org/apache/commons/math3/util/FastMath.java
>> ----------------------------------------------------------------------
>> diff --git a/src/main/java/org/apache/commons/math3/util/FastMath.java
>> b/src/main/java/org/apache/commons/math3/util/FastMath.java
>> index 5178215..09c5976 100644
>> --- a/src/main/java/org/apache/commons/math3/util/FastMath.java
>> +++ b/src/main/java/org/apache/commons/math3/util/FastMath.java
>> @@ -876,7 +876,11 @@ public class FastMath {
>> if (x < 0.0) {
>> intVal = (int) -x;
>>
>> - if (intVal > 746) {
>> + // TEMP: special handling of negative_infinity
>> + // the above might fail in non-reproducible ways with Sun JDK
>> 1.5,
>> + // most likely due to a bug in the JIT. Add a safe-guard for
>> very
>> + // negative numbers.
>> + if (intVal > 746 || x < Integer.MIN_VALUE) {
>> if (hiPrec != null) {
>> hiPrec[0] = 0.0;
>> hiPrec[1] = 0.0;
>>
>>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org