floating point FORMAT

[Nikodemus Siivola]

  (format nil "~4,1F" 0.001) => "0.00" (should be " 0.0");
  (format nil "~4,1@F" 0.001) => "+.00" (should be "+0.0").
  (format nil "~E" 0.01) => "10.e-3" (should be "1.e-2");
  (format nil "~G" 0.01) => "10.e-3" (should be "1.e-2");

[David Vázquez (davazp)]

I attach a patch for these bugs.

The first bug, corresponding to two first expressions, it is due to flonum-to-digits tries to accept positive numbers,
however when it rounds, it could be zero. I fix flonum-to-digits to accept non-negative numbers.

The second bug, corresponding to two last expressions, it is due to format-exp-aux assumes the value returned
from scale-exponent is in (0.1, 1.0), though 1.0 could be reached.

[Roman Marynchak (roman-marynchak)]

Hi David,

I briefly reviewed the patches, and they look good.

Two notes which I can give are based on my own SBCL patching experience (something like "newbie to newbie"):

1. It would be nice to add some simple regression tests in /tests/print.impure.lisp (you can find the examples there as well).
2. You probably should change the bug status back to 'Confirmed'. 'Fix Committed' means that the patch is already in SBCL source tree.

Regards,
Roman

[Nikodemus Siivola (nikodemus)]

Thank you!

Test cases are sorely needed, yeah. Some random tests would be good too, and do much to increase confidence in the code. Apropos, did you check with ANSI test suite that this doesn't introduce new failures?

More detailed comments about why the fix is right would also be very welcome: reviewing the FP printing code is a pain, and likely to happen the faster the more detailed the explanation of changes is.

[David Vázquez (davazp)]

All tests in tests/ directory are passed after of the patch. I include the following tests in tests/print.impure.lisp. This file contains several float point printing tests, so I think it is not required to add more ones.

;;; Tests for bug lp#308961
(assert (string= (format nil "~4,1F" 0.001) " 0.0"))
(assert (string= (format nil "~4,1@F" 0.001) "+0.0"))
(assert (string= (format nil "~E" 0.01) "1.e-2"))
(assert (string= (format nil "~G" 0.01)) "1.00e-2")

About a deeper explication. Well, as I said, the first bug is located in flonum-to-digits function, which is a translation of a Scheme code. The algorithm implemented is supposed to work with positive numbers. However, the function was expanded in order to support rounding its input. So, since 0.001 is round to 0.0, the implemented algorithm fails. This require to understand the algorithm, but I test the lower limit of the interval (r - m-) is positive simply, in order not to generate extra zeros.

The second bug is simpler to explain. format-exp-aux scales a number such that it will have K digits before of the decimal point. This relies on scale-exponent. A example shows the trouble:

(format nil "~E" 0.1)
  0: (FORMAT-EXP-AUX #<SB-IMPL::STRING-OUTPUT-STREAM {AFE5DE9}> 0.1 NIL NIL NIL 1 NIL #\ NIL NIL)
    1: (SB-IMPL::SCALE-EXPONENT 0.1)
    1: SB-IMPL::SCALE-EXPONENT returned 0.1 0
  0: FORMAT-EXP-AUX returned "1"

0.1 is 0.1 * 10^0 according to scale-exponent. Then, it is multiplied by 10^K before printing. This works well.

(format nil "~E" 0.01)
  0: (FORMAT-EXP-AUX #<SB-IMPL::STRING-OUTPUT-STREAM {AA744C1}> 0.01 NIL NIL NIL 1 NIL #\ NIL NIL)
    1: (SB-IMPL::SCALE-EXPONENT 0.01)
    1: SB-IMPL::SCALE-EXPONENT returned 1.0 -2
  0: FORMAT-EXP-AUX returned "2"

In this example, scale-exponent returns 1.0 and -2, meaning 0.01 = 1.0 * 10^-2. Again, format-aux-exp multiply it by 10^K. But as 1.0 has a digit before of the decimal point, the result will have K+1 digits.

This is due to format-exp-aux assumed the returned value will be lesser to 1.0. In order to fix this, I decrement K when scale-exponent return 1.0 as primary value.

Greetings,
David.

[Nikodemus Siivola (nikodemus)]

Thank you! Sorry about taking so long to merge this.

Committed as:

commit 58187b3f2ab87bce54657c9c94ac2b3090103ba1
Author: David Vázquez <xxx>
Date: Mon Jun 28 17:28:08 2010 +0200

    FLONUM-TO-DIGITS handles non-negative input properly

     lp#308961, part 1.

     SBCL has an extended version of the Burger & Dybwig fp printer, which
     supports rounding.

     It did not however support zero -- but in eg.

       (format nil "~,1F" 0.001)

     0.001 is rounded to zero, which leads FLONUM-TO-DIGITS giving us one zero too
     many. Ie. it should be

        => "0.0"

     but prior to this we got "0.00" instead.

     This patch removes the special casing for 0, and instead tests that the lower
     limit of the interval (- r m-) is positive in order not to generate extra
     zeros.

and

commit dafa18aa6bd65fe2129a32b0e827141684bb159a
Author: David Vázquez <xxx>
Date: Mon Jun 28 18:53:30 2010 +0200

    FORMAT-AUX-EXP: adjust scale if scale-exponent return 1.0

     lp#308961, part 2.

     FORMAT-EXP-AUX scales a number such that it will have K digits before of the
     decimal point. This relies on scale-exponent.

      (format nil "~E" 0.1) ; => "1.e-1"

      0: (FORMAT-EXP-AUX #<SB-IMPL::STRING-OUTPUT-STREAM {AFE5DE9}> 0.1 NIL NIL
                         NIL 1 NIL #\ NIL NIL)
        1: (SB-IMPL::SCALE-EXPONENT 0.1)
        1: SB-IMPL::SCALE-EXPONENT returned 0.1 0
      0: FORMAT-EXP-AUX returned "1"

     0.1 is 0.1 * 10^0 according to scale-exponent. Then, it is multiplied by 10^K
     before printing. Everything works out fine.

     However!

      (format nil "~E" 0.01) ; => "10.e-3" ... oops

      0: (FORMAT-EXP-AUX #<SB-IMPL::STRING-OUTPUT-STREAM {AA744C1}> 0.01 NIL NIL
                         NIL 1 NIL #\ NIL NIL)
        1: (SB-IMPL::SCALE-EXPONENT 0.01)
        1: SB-IMPL::SCALE-EXPONENT returned 1.0 -2
      0: FORMAT-EXP-AUX returned "2"

     In this example, scale-exponent returns 1.0 and -2, meaning 0.01 = 1.0 *
     10^-2. Again, format-aux-exp multiply it by 10^K. But as 1.0 has a digit
     before of the decimal point, the result will have K+1 digits.

     This is due to format-exp-aux assumed the returned value will be lesser to
     1.0. In order to fix this, we decrement K when scale-exponent return 1.0 as
     primary value.

with tests in

commit bea52212cb688209f31804bc6d4644968ac28246
Author: Nikodemus Siivola <email address hidden>
Date: Tue Jun 7 12:29:21 2011 +0300

    tests and NEWS for lp#308961

      Also add a random FP read/print consistency tester.

      For now it skips denormalized double-floats due to bug 793774.