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 <seb...@gmail.com> wrote:
> On 22 January 2015 at 01:59, Phil Steitz <phil.ste...@gmail.com> wrote:
> > On 1/21/15 4:56 PM, sebb wrote:
> >> On 21 January 2015 at 20:39, Thomas Neidhart <thomas.neidh...@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, <t...@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.neidh...@gmail.com>
> >>>>>> Authored: Wed Jan 21 00:42:16 2015 +0100
> >>>>>> Committer: Thomas Neidhart <thomas.neidh...@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-unsubscr...@commons.apache.org
> >>>> For additional commands, e-mail: dev-h...@commons.apache.org
> >>>>
> >>>
> >>> ---------------------------------------------------------------------
> >>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> >>> For additional commands, e-mail: dev-h...@commons.apache.org
> >>>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> >> For additional commands, e-mail: dev-h...@commons.apache.org
> >>
> >>
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> > For additional commands, e-mail: dev-h...@commons.apache.org
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
>
>
