JDK-8010430 : Math.round has surprising behavior for odd values of ulp 1
  • Type: Bug
  • Component: core-libs
  • Sub-Component: java.lang
  • Affected Version: 7
  • Priority: P3
  • Status: Closed
  • Resolution: Fixed
  • OS: generic
  • CPU: generic
  • Submitted: 2013-03-20
  • Updated: 2017-05-17
  • Resolved: 2013-09-12
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 8
8 b108Fixed
Related Reports
Relates :  
Description
FULL PRODUCT VERSION :
1.7.0_11-b21

ADDITIONAL OS VERSION INFORMATION :
generic

EXTRA RELEVANT SYSTEM CONFIGURATION :
generic

A DESCRIPTION OF THE PROBLEM :
Since bug 6430675 fix, Math.round(float) and Math.round(double)
specs no longer specify rounding by adding 0.5 and then using floor,
and let the user believe that they  " now round up properly " .

However these methods still exhibit some suprising results coming
from adding 0.5 and then using floor, namely when using odd values of ulp 1, which are mathematical integers but for which Math.round return value+1.

For float case, these are odd values in [8388609.0f,16777215.0f] and
[-16777215.0f,-8388609.0f], i.e. [(1<<23)+1,(1<<24)-1] and [-((1<<24)-1),-((1<<23)+1)].

This was a known limitation of previous implementation (I remember creating a bug for it around 2007, but it was rejected as comforming to spec.).

Either the references to floor(value+0.5) should be restored in the specs, or the implementations should be aligned to their new spec.


STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
Call Math.round(value) with an odd float or double value of ulp 1


EXPECTED VERSUS ACTUAL BEHAVIOR :
EXPECTED -
Math.round(value) == value

ACTUAL -
Math.round(value) == value+1


REPRODUCIBILITY :
This bug can be reproduced always.

---------- BEGIN SOURCE ----------
        for (float sign : new float[]{-1,1}) {
            for (float v1 : new float[]{1<<23,1<<24}) {
                for (int k=-5;k<=5;k++) {
                    float value = (v1+k)*sign;
                    float actual = Math.round(value);
                    if (actual != value) {
                        System.out.println( " Math.round( " +value+ " ) =  " +actual+ "  (unexpected) " );
                    } else {
                        System.out.println( " Math.round( " +value+ " ) =  " +actual);
                    }
                }
            }
        }
        System.out.println();
        for (double sign : new double[]{-1,1}) {
            for (double v1 : new double[]{1L<<52,1L<<53}) {
                for (int k=-5;k<=5;k++) {
                    double value = (v1+k)*sign;
                    double actual = Math.round(value);
                    if (actual != value) {
                        System.out.println( " Math.round( " +value+ " ) =  " +actual+ "  (unexpected) " );
                    } else {
                        System.out.println( " Math.round( " +value+ " ) =  " +actual);
                    }
                }
            }
        }
---------- END SOURCE ----------

CUSTOMER SUBMITTED WORKAROUND :
// Implementation and JUnit test for double case:

    public static long round(double value) {
        final int exponent = Math.getExponent(value);
        if (exponent < 0) {
            // abs(value) < 1.
            if (value < -0.5) {
                return -1;
            } else if (value < 0.5) {
                return 0;
            } else {
                return 1;
            }
        } else if (exponent < 52) {
            final long matIntPart = (long)value;
            final double intToValue = value-(double)matIntPart;
            if (intToValue < -0.5) {
                return matIntPart - 1;
            } else if (intToValue < 0.5) {
                return matIntPart;
            } else {
                return matIntPart + 1;
            }
        } else {
            // Infinity, NaN, or a mathematical integer.
            return (long)value;
        }
    }
    
    public static boolean isMathematicalInteger(double value) {
        final double valueAbs = Math.abs(value);
        return ((valueAbs >= (double)(1L<<52))
                && (valueAbs != Double.POSITIVE_INFINITY))
                || (value == (double)(long)value);
    }
    
    public void test_round_double() {
        final Random random = new Random(123456789L);
        
        assertEquals(0L, round(Double.NaN));
        assertEquals(Long.MIN_VALUE, round(Double.NEGATIVE_INFINITY));
        assertEquals(Long.MAX_VALUE, round(Double.POSITIVE_INFINITY));

        for (int k=0;k<10;k++) {
            for (double value : new double[]{(1L<<52)+k,-(1L<<52)-k}) {
                assertEquals(((long)value), round(value));
            }
        }

        for (int i=0;i<100*1000;i++) {
            // Uniform value and magnitude.
            final long longValue = (random.nextLong() >> random.nextInt(64));

            // Either exact, or of smaller magnitude due to mantissa
            // overflow, so still a mathematical integer.
            final double refValue = (double)longValue;
            
            final double[] v1Tab = new double[]{refValue, refValue-0.5, refValue+0.5, refValue+random.nextDouble()};
            for (double v1 : v1Tab) {
                final double v1Bef = Math.nextAfter(v1, Double.NEGATIVE_INFINITY);
                final double v1Aft = Math.nextAfter(v1, Double.POSITIVE_INFINITY);
                final double[] v2Tab = new double[]{v1, v1Bef, v1Aft};
                for (double value : v2Tab) {
                    final long expected;
                    if (isMathematicalInteger(value)) {
                        expected = (long)value;
                    } else {
                        final boolean neg = (value < 0);
                        final boolean equidistant = isMathematicalInteger(value+value);
                        if (equidistant) {
                            final long lowerMag = (long)value;
                            expected = (neg ? lowerMag : lowerMag+1);
                        } else {
                            if (neg) {
                                final double distFromBef = value - (double)(((long)value)-1);
                                final double distFromAft = (double)(long)value - value;
                                expected = (distFromBef < distFromAft) ? ((long)value)-1 : (long)value;
                            } else {
                                final double distFromBef = value - (double)(long)value;
                                final double distFromAft = (double)(((long)value)+1) - value;
                                expected = (distFromBef < distFromAft) ? (long)value : ((long)value)+1;
                            }
                        }
                    }
                    final long actual = round(value);
                    if (expected != actual) {
                        System.out.println( " value =  " +value);
                    }
                    assertEquals(expected, actual);
                }
            }
        }
    }
Comments
Yes, the specification should be clarified for the ranges in question. See discussion on core-libs-dev: http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-January/014129.html
21-03-2013