* bug#51276: Problems with format and scaling floats
2021-10-18 21:22 bug#51276: Problems with format and scaling floats Timothy Sample
@ 2021-10-19 1:02 ` lloda
2021-10-19 8:42 ` Bengt Richter
1 sibling, 0 replies; 3+ messages in thread
From: lloda @ 2021-10-19 1:02 UTC (permalink / raw)
To: Timothy Sample; +Cc: 51276
Thank you very much, patch applied!
Now that you've dived into (ice-9 format), what about this one:
(format #t "~,,2f~%" 0.01)
=> 1.0
(format #t "~,,3f~%" 0.01) ; !!
=> 010.0
(format #t "~,,3f~%" 0.001)
=> 1.0
(format #t "~,,4f~%" 0.001) ; !!
=> 0010.0
(format #t "~,,4f~%" 0.0001)
=> 1.0
(format #t "~,,5f~%" 0.0001) ; ok somehow...
=> 10.0
regards
> On 18 Oct 2021, at 23:22, Timothy Sample <samplet@ngyro.com> wrote:
>
> Hi Guilers,
>
> It turns out there’s a little blunder in ‘format’ (from ‘ice-9’). Look
> at what happens when using the SCALE argument to format a fixed-point
> float (this is Guile from the Git repo at the time of writing):
>
> GNU Guile 3.0.7.6-22120
> Copyright (C) 1995-2021 Free Software Foundation, Inc.
>
> Guile comes with ABSOLUTELY NO WARRANTY; for details type `,show w'.
> This program is free software, and you are welcome to redistribute it
> under certain conditions; type `,show c' for details.
>
> Enter `,help' for help.
> scheme@(guile-user)> (format #t "~,,3f~%" 0.00123)
> 0.23
> $3 = #t
> scheme@(guile-user)> (format #t "~,,1f~%" 0.00123)
> ice-9/boot-9.scm:1685:16: In procedure raise-exception:
> Value out of range 0 to 400: -1
>
> Entering a new prompt. Type `,bt' for a backtrace or `,q' to continue.
>
> The first example gives the wrong result. Scaling 0.00123 by 3 should
> yield 1.23, not 0.23. For the second example, instead of 0.0123, we get
> an error! What’s going on here?
>
> Well, our ‘format’ code comes from SLIB and was written in 1998, so it’s
> not easy to explain. There’s so much mutation even a C programmer would
> blush! ;) The issue happens in the ‘format:parse-float’ procedure
> (which is defined inside of ‘format’). It normalizes the string
> representation of a number, and applies the scale argument when needed.
> It does this by keeping a string of digits and the location of the
> decimal point. Another thing it keeps track of the leading zeros in a
> variable called ‘left-zeros’. Here’s the code that does the final
> shifting and places the decimal point:
>
> (if (> left-zeros 0)
> (if (<= left-zeros shift) ; shift always > 0 here
> (format:fn-shiftleft shift) ; shift out 0s
> (begin
> (format:fn-shiftleft left-zeros)
> (set! format:fn-dot (- shift left-zeros))))
> (set! format:fn-dot (+ format:fn-dot shift)))
>
> The issue is that the cases in the inner ‘if’ form are reversed. That
> is, if there are MORE leading zeros than we need to shift, we can just
> shift. Otherwise (if there are FEWER leading zeros), we need to shift
> out the zeros and then move the decimal point (‘format:fn-dot’).
>
> AFAICS, this bug was in the original SLIB implementation (1998) and has
> not been fixed since then. It’s been in Guile since 1999.
>
> Anyway, that’s more than anyone cares to know.... Here’s a patch with
> tests! :)
>
> From c31d1f5d44343da1201ea1be86bc6b2ac8af6c8d Mon Sep 17 00:00:00 2001
> From: Timothy Sample <samplet@ngyro.com>
> Date: Mon, 18 Oct 2021 17:07:41 -0400
> Subject: [PATCH] ice-9 format: Fix scaling floats with leading zeros
>
> ---
> module/ice-9/format.scm | 4 ++--
> test-suite/tests/format.test | 10 ++++++++--
> 2 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/module/ice-9/format.scm b/module/ice-9/format.scm
> index 48d9c0c84..ee7cba910 100644
> --- a/module/ice-9/format.scm
> +++ b/module/ice-9/format.scm
> @@ -1359,10 +1359,10 @@
> (else
> (if (> left-zeros 0)
> (if (<= left-zeros shift) ; shift always > 0 here
> - (format:fn-shiftleft shift) ; shift out 0s
> (begin
> (format:fn-shiftleft left-zeros)
> - (set! format:fn-dot (- shift left-zeros))))
> + (set! format:fn-dot (- shift left-zeros)))
> + (format:fn-shiftleft shift)) ; shift out 0s
> (set! format:fn-dot (+ format:fn-dot shift))))))))
>
> (let ((negexp ; expon format m.nnnEee
> diff --git a/test-suite/tests/format.test b/test-suite/tests/format.test
> index b9aa7a854..d5111f1c6 100644
> --- a/test-suite/tests/format.test
> +++ b/test-suite/tests/format.test
> @@ -2,7 +2,7 @@
> ;;;; Matthias Koeppe <mkoeppe@mail.math.uni-magdeburg.de> --- June 2001
> ;;;;
> ;;;; Copyright (C) 2001, 2003, 2004, 2006, 2010, 2011, 2012,
> -;;;; 2014, 2017 Free Software Foundation, Inc.
> +;;;; 2014, 2017, 2021 Free Software Foundation, Inc.
> ;;;;
> ;;;; This library is free software; you can redistribute it and/or
> ;;;; modify it under the terms of the GNU Lesser General Public
> @@ -121,7 +121,13 @@
> ;; in guile prior to 1.6.9 and 1.8.1, leading zeros were incorrectly
> ;; stripped, moving the decimal point and giving "25.0" here
> (pass-if "string 02.5"
> - (string=? "2.5" (format #f "~f" "02.5"))))
> + (string=? "2.5" (format #f "~f" "02.5")))
> +
> + (pass-if "scale with few leading zeros"
> + (string=? "1.23" (format #f "~,,3f" "0.00123")))
> +
> + (pass-if "scale with many leading zeros"
> + (string=? "0.0123" (format #f "~,,1f" "0.00123"))))
>
> ;;;
> ;;; ~h
> --
> 2.33.0
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* bug#51276: Problems with format and scaling floats
2021-10-18 21:22 bug#51276: Problems with format and scaling floats Timothy Sample
2021-10-19 1:02 ` lloda
@ 2021-10-19 8:42 ` Bengt Richter
1 sibling, 0 replies; 3+ messages in thread
From: Bengt Richter @ 2021-10-19 8:42 UTC (permalink / raw)
To: Timothy Sample; +Cc: 51276
Nice catch :)
On +2021-10-18 17:22:49 -0400, Timothy Sample wrote:
> Hi Guilers,
>
> It turns out there’s a little blunder in ‘format’ (from ‘ice-9’). Look
> at what happens when using the SCALE argument to format a fixed-point
> float (this is Guile from the Git repo at the time of writing):
>
> GNU Guile 3.0.7.6-22120
> Copyright (C) 1995-2021 Free Software Foundation, Inc.
>
> Guile comes with ABSOLUTELY NO WARRANTY; for details type `,show w'.
> This program is free software, and you are welcome to redistribute it
> under certain conditions; type `,show c' for details.
>
> Enter `,help' for help.
> scheme@(guile-user)> (format #t "~,,3f~%" 0.00123)
> 0.23
> $3 = #t
> scheme@(guile-user)> (format #t "~,,1f~%" 0.00123)
> ice-9/boot-9.scm:1685:16: In procedure raise-exception:
> Value out of range 0 to 400: -1
>
> Entering a new prompt. Type `,bt' for a backtrace or `,q' to continue.
>
> The first example gives the wrong result. Scaling 0.00123 by 3 should
> yield 1.23, not 0.23. For the second example, instead of 0.0123, we get
> an error! What’s going on here?
>
> Well, our ‘format’ code comes from SLIB and was written in 1998, so it’s
> not easy to explain. There’s so much mutation even a C programmer would
> blush! ;) The issue happens in the ‘format:parse-float’ procedure
> (which is defined inside of ‘format’). It normalizes the string
> representation of a number, and applies the scale argument when needed.
> It does this by keeping a string of digits and the location of the
> decimal point. Another thing it keeps track of the leading zeros in a
> variable called ‘left-zeros’. Here’s the code that does the final
> shifting and places the decimal point:
>
> (if (> left-zeros 0)
> (if (<= left-zeros shift) ; shift always > 0 here
> (format:fn-shiftleft shift) ; shift out 0s
> (begin
> (format:fn-shiftleft left-zeros)
> (set! format:fn-dot (- shift left-zeros))))
> (set! format:fn-dot (+ format:fn-dot shift)))
>
> The issue is that the cases in the inner ‘if’ form are reversed. That
> is, if there are MORE leading zeros than we need to shift, we can just
> shift. Otherwise (if there are FEWER leading zeros), we need to shift
> out the zeros and then move the decimal point (‘format:fn-dot’).
>
> AFAICS, this bug was in the original SLIB implementation (1998) and has
> not been fixed since then. It’s been in Guile since 1999.
>
> Anyway, that’s more than anyone cares to know.... Here’s a patch with
> tests! :)
>
1999 until now (2021), /and/ reliably reproduced all that time! :)
--
Regards,
Bengt Richter
^ permalink raw reply [flat|nested] 3+ messages in thread