Good catch!

It looks to me like the root cause is that the argument 2.3 is being
treated as a floating point value, whereas in this context it is really
intended as a format specifier (i.e. a string, such as you might use
in a sprintf() function).

I wrote a test to document this bug, and added the fix to Squeak trunk.
The test and the modified #printPaddedWith:to: method are in the attached
change set.

Dave

> Hi, using formatted number output recently I got the following output:
> 
> 1.0 printPaddedWith: $0 to: 2.1. "'01.0'"
> 1.0 printPaddedWith: $0 to: 2.2. "'01.00'"
> 1.0 printPaddedWith: $0 to: 2.3. "'01.00'"    <-- wrong
> 1.0 printPaddedWith: $0 to: 2.4. "'01.000'"   <-- wrong
> 1.0 printPaddedWith: $0 to: 2.5. "'01.00000'"
> 
> running Float>>printPaddedWith:to: in the debugger showed me that the reason
> of this odd behaviour seems to be produced by Float>>truncated and/or
> Float>>fractionPart.  I can provoke errors of that kind even easier:
> 
> (2.3 fractionPart * 10) truncated. => "2" (instead of 3)
> 
> In Primitive 51 (primitiveTruncated for Float>>truncated) I read
> 
> frac = modf(rcvr, &trunc);
> 
> so the problem looks like an issue with rounding errors in Math-Library's
> modf
> function.
> 
> Indeed, I can reproduce the odd result of "(2.3 fractionPart * 10)
> truncated."
> in C++ using modf:
> 
> // g++ -lm truncate.cpp && ./a.out
> #include<iostream>
> #include<cmath>
> using namespace std;
> int main(){
>   double f1, f2, f3;
>   modf(10 * 0.3, &f1);
>   cout << f1 << endl;
>   modf(10 * modf(2.3, &f2), &f3);
>   cout << f3 << endl;
> }
> // output:
> // 3 (ok)
> // 2 (error - should be 3)
> 
> Question: what can be done about this?
> 
> As a matter of fact, several important methods spread over a number of
> classes
> (Float, Number, Morph ... being among them) are senders of Float>>truncated
> or
> Float>>fractionPart, so rounding errors in primitive 51 or 52 could provoke
> unexpected behaviour in surprising places.
> 
> 
> -- 
> View this message in context: 
> http://forum.world.st/rounding-errors-tp3327130p3327130.html
> Sent from the Pharo Smalltalk Users mailing list archive at Nabble.com.
'From Squeak4.2 of 26 February 2011 [latest update: #11044] on 27 February 2011 
at 9:55:52 pm'!
"Change Set:            Float-printPaddedWith-dtl
Date:                   27 February 2011
Author:                 David T. Lewis

A test and a fix for the bug reported in 
http://lists.gforge.inria.fr/pipermail/pharo-users/2011-February/001569.html";!


!Float methodsFor: 'printing' stamp: 'dtl 2/27/2011 21:39'!
printPaddedWith: aCharacter to: aNumber 
        "Answer the string containing the ASCII representation of the receiver 
        padded on the left with aCharacter to be at least on aNumber 
        integerPart characters and padded the right with aCharacter to be at 
        least anInteger fractionPart characters."
        | aStream digits fPadding fLen iPadding iLen curLen periodIndex |
        #Numeric.
        "2000/03/04  Harmon R. Added Date and Time support"
        aStream := WriteStream on: (String new: 10).
        self printOn: aStream.
        digits := aStream contents.
        periodIndex := digits indexOf: $..
        curLen := periodIndex - 1.
        iLen := aNumber integerPart.
        curLen < iLen
                ifTrue: [iPadding := (String new: (iLen - curLen) asInteger) 
atAllPut: aCharacter;
                                         yourself]
                ifFalse: [iPadding := ''].
        curLen := digits size - periodIndex.
        "n.b. Treat aNumber as a string format specifier rather than as a 
number, because
        floating point truncation can produce incorrect results for the 
fraction part."
        fLen := (aNumber asString copyAfterLast: $. )
                ifNotEmpty: [:s | s asInteger]
                ifEmpty: [ 0 ].
        curLen < fLen
                ifTrue: [fPadding := (String new: fLen - curLen) atAllPut: 
aCharacter;
                                         yourself]
                ifFalse: [fPadding := ''].
        ^ iPadding , digits , fPadding! !


!FloatTest methodsFor: 'printing' stamp: 'dtl 2/27/2011 21:01'!
testPrintPaddedWithTo
        "This bug was reported in 
http://lists.gforge.inria.fr/pipermail/pharo-users/2011-February/001569.html.
        The problem was caused by treating the format specifier as a number 
rather than
        as a string, such the the number may be a Float subject to floating 
point rounding
        errors. The solution to treat the format specifier as a string, and 
extract the integer
        fields before and after the decimal point in the string."

        self assert: [(1.0 printPaddedWith: $0 to: 2.2) = '01.00'].
        self assert: [(1.0 printPaddedWith: $X to: 2.2) = 'X1.0X'].
        self assert: [(1.0 printPaddedWith: $0 to: 2) = '01.0'].
        self assert: [(12345.6789 printPaddedWith: $0 to: 2) = '12345.6789'].
        self assert: [(12345.6789 printPaddedWith: $0 to: 2.2) = '12345.6789'].
        self assert: [(12.34 printPaddedWith: $0 to: 2.2) = '12.34'].
        self assert: [(12345.6789 printPaddedWith: $0 to: 2.2) = '12345.6789'].
        self assert: [(123.456 printPaddedWith: $X to: 4.4) = 'X123.456X'].
        self assert: [(1.0 printPaddedWith: $0 to: 2.1) = '01.0'].
        self assert: [(1.0 printPaddedWith: $0 to: 2.2) = '01.00'].
        self assert: [(1.0 printPaddedWith: $0 to: 2.3) = '01.000']. 
"previously failed due to float usage"
        self assert: [(1.0 printPaddedWith: $0 to: 2.4) = '01.0000']. 
"previously failed due to float usage"
        self assert: [(1.0 printPaddedWith: $0 to: 2.5) = '01.00000']

! !

Reply via email to