Skip to content

[Java]: Too restrictive condition on dvIsTrailingZeros when e2 < 0 results in wrong rounding. #243

@tanishiking

Description

@tanishiking

When e2 < 0, current RyuFloat calculates dvIsTrailingZeros by the code below, which should be equivalent to mv%2^(q-1) == 0.

dvIsTrailingZeros = (q < FLOAT_MANTISSA_BITS) && (mv & ((1 << (q - 1)) - 1)) == 0;

dvIsTrailingZeros = (q < FLOAT_MANTISSA_BITS) && (mv & ((1 << (q - 1)) - 1)) == 0;

If I understand correctly, (q < FLOAT_MANTISSA_BITS) is a cap for q that works as follows:

  • The maximum value of v is 2^FLOAT_MANTISSA_BITS - 1
  • Therefore, when q is greater than or equal to FLOAT_MANTISSA_BITS, v is never divisible by 2^q
  • So, if (q < FLOAT_MANTISSA_BITS) is false, we can skip the (mv & ((1 << (q - 1)) - 1)) == 0 check

However, I think it should be (q - 1 < FLOAT_MANTISSA_BITS + 2) because:

  • We're checking if mv is divisible by 2^(q-1), not 2^q. So the left-hand side should be q - 1
  • The maximum value of mv is larger than 2^FLOAT_MANTISSA_BITS - 1:
    • mv is 4 * m, where m is the 23-bit mantissa. This means mv <= 4 * (2^23 - 1) = 2^25 - 4
    • To check if mv is divisible by 2^(q-1), the condition q < 23 is too restrictive, because mv can be divisible by up to 2^24
    • Therefore, the right-hand side should be FLOAT_MANTISSA_BITS + 2

Actually, a test from f2s_test.cc's lotsOfTrailingZeros fails in the Java implementation:

ASSERT_F2S("2.4414062E-3", 2.4414062E-3f);

1) lotsOfTrailingZeros(info.adams.ryu.RyuFloatTest)
org.junit.ComparisonFailure: expected:<0.002441406[2]> but was:<0.002441406[3]>

This is because:

  • mv = 41943040, e2 = -34, and q = 23
  • Therefore, dvIsTrailingZeros = false because q < 23 is false (but it should be true)
  • By step 4, dv is reduced from 244140625 to 24414062 (with lastRemovedDigit = 5)
  • The condition (dvIsTrailingZeros && (lastRemovedDigit == 5) && (dv % 2 == 0)) evaluates to false (but it should be true!), because dvIsTrailingZeros = false
    if (dvIsTrailingZeros && (lastRemovedDigit == 5) && (dv % 2 == 0)) {
    // Round down not up if the number ends in X50000 and the number is even.
    lastRemovedDigit = 4;

    As a result, it is rounded up to 24414063, but it should be rounded to 24414062.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions