Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.9
    • Fix Version/s: 3.1
    • Component/s: None
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      Underflow detection in small floats has a bug, and can incorrectly result in a byte value of 0 for a non-zero float.

        Activity

        Hide
        Yonik Seeley added a comment -

        1.9 is the earliest "affects" version we have in JIRA, but this underflow detection issue goes all the way back to the original floatToByte code added in v1.3

        The problem is that the code only checked for underflow by checking the exponent:

            if (exponent < 0) {                           // underflow: use min value
              exponent = 0;
              mantissa = 1;
            }
        

        But it's also underflow if the exponent is 0 and the mantissa bits are also 0.
        So for the old code, this should be:

            if (exponent < 0 || exponent == 0 && mantissa == 0) {  // underflow: use min value
              exponent = 0;
              mantissa = 1;
            }
        

        Now the trick is to find the most efficient way to make an equivalent fix SmallFloat

        Show
        Yonik Seeley added a comment - 1.9 is the earliest "affects" version we have in JIRA, but this underflow detection issue goes all the way back to the original floatToByte code added in v1.3 The problem is that the code only checked for underflow by checking the exponent: if (exponent < 0) { // underflow: use min value exponent = 0; mantissa = 1; } But it's also underflow if the exponent is 0 and the mantissa bits are also 0. So for the old code, this should be: if (exponent < 0 || exponent == 0 && mantissa == 0) { // underflow: use min value exponent = 0; mantissa = 1; } Now the trick is to find the most efficient way to make an equivalent fix SmallFloat
        Hide
        Yonik Seeley added a comment -

        Here's a patch - the fix to SmallFloat turned out easier than I thought, and since we are working with the exponent and mantissa together, doesn't slow down the code at all (the less-than is just changed to a less-than-or-equals).

        I also re-ran TestSmallFloat.testAllFloats() to both verify that SmallFloat matches the original floatToByte after that was also fixed, and verified that no positive float maps to byte #0

        Show
        Yonik Seeley added a comment - Here's a patch - the fix to SmallFloat turned out easier than I thought, and since we are working with the exponent and mantissa together, doesn't slow down the code at all (the less-than is just changed to a less-than-or-equals). I also re-ran TestSmallFloat.testAllFloats() to both verify that SmallFloat matches the original floatToByte after that was also fixed, and verified that no positive float maps to byte #0
        Hide
        Robert Muir added a comment -

        Thanks Yonik. I tested the patch, this solves the issues i have been seeing with my silly tests (they are not very interesting, just weaker versions of TestAllFloats looking for f>0 && b==0)

        Show
        Robert Muir added a comment - Thanks Yonik. I tested the patch, this solves the issues i have been seeing with my silly tests (they are not very interesting, just weaker versions of TestAllFloats looking for f>0 && b==0)
        Hide
        Grant Ingersoll added a comment -

        Bulk close for 3.1

        Show
        Grant Ingersoll added a comment - Bulk close for 3.1

          People

          • Assignee:
            Unassigned
            Reporter:
            Yonik Seeley
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development