On 22 January 2015 at 11:44, 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?
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, <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
