commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sebb <seb...@gmail.com>
Subject Re: [math] Add temporary check for rare test failure.
Date Thu, 22 Jan 2015 11:44:37 GMT
On 22 January 2015 at 01:59, Phil Steitz <phil.steitz@gmail.com> wrote:
> On 1/21/15 4:56 PM, sebb wrote:
>> On 21 January 2015 at 20:39, Thomas Neidhart <thomas.neidhart@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 <thomas.neidhart@gmail.com>
>>>>>> Authored: Wed Jan 21 00:42:16 2015 +0100
>>>>>> Committer: Thomas Neidhart <thomas.neidhart@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


Mime
View raw message