"Wrong" output for double types

[josesimoes]

Hello,

At .NET nanoFramework we've been using your awesome library for some time now.
Currently migrating to the latest version.

Something that I would like your advice on is the "wrong" output for some double values when using the format string %0.19G.
Here's the output of a for loop of doubles (right hand side is the equivalent as int, for reference):

0 0
1 1
2 2
3 3
4 4
5 5
6 6
7 7
8 8
9 9
10 10
11 11
12 12
13 13
14 13.999999999999999
15 15
16 16
17 17
18 18
19 19
20 20
21 21
22 22
23 22.999999999999998
24 23.999999999999999
25 25
26 26
27 27
28 27.999999999999998
29 29
30 30
31 31
32 32
33 32.999999999999998
34 33.999999999999999
35 35
36 36
37 37
38 37.999999999999998
39 39
40 40
41 40.999999999999996
42 42
43 42.999999999999998
44 44
45 45
46 45.999999999999996
47 47
48 47.999999999999998
49 49
50 50
51 50.999999999999996
52 52
53 52.999999999999998
54 54
55 55
56 55.999999999999996
57 57
58 57.999999999999998
59 59
60 60
61 60.999999999999996
62 62
63 62.999999999999998
64 64
65 65
66 65.999999999999996
67 67
68 67.999999999999998
69 69
70 70
71 70.999999999999996
72 72
73 72.999999999999998
74 74
75 75
76 75.999999999999996
77 77
78 77.999999999999998
79 79
80 80
81 80.999999999999996
82 81.999999999999993
83 83
84 84
85 85
86 85.999999999999996
87 86.999999999999993
88 88
89 89
90 90
91 90.999999999999996
92 91.999999999999993
93 93
94 94
95 95
96 95.999999999999996
97 96.999999999999993
98 98
99 99

Any idea on what could be causing this rounding error, and how to fix it?
Thanks!

[skirridsystems]

Hi, sorry for the delay, I've been away on holiday. Looks like you already found another implementation which works better for you.

I will take a look at this but to be honest the focus has been on simplicity and minimum code space rather than accuracy at 19 significant digits; that's pretty close to the limit for doubles. The code in newlib-nano looks to be much more thorough than mine, so no surprise that it gets better results. But I'm surprised that it compiles smaller, just because there is a lot more of it.

I'll see if there is a simple fix for integer values that doesn't inflate the code size too much.

[josesimoes]

Hi Simon,

Thanks for looking into this.
Yes, that's pretty close to the limit 😅 specially when testing the MAX and MIN for double!
I'm OK with reverting back to es-printf if it works and the code size is equivalent.

Maybe we're seeing a smaller binary because of the aggressing linker optimizations on the release build...

[josesimoes]

Simon, looks like there are some edge cases that the newlib nano is not working as es-print was...
So... I can either revert back to the version we had working before, or I can reopen the PR to migrate to v1.3 providing there a nice way to have it working near the edge. I'll wait for your feedback, at your convenience, of course.

[skirridsystems]

When you say 'the version we had working before' is this a regression in es-printf from 1.2 or were you using something different again? Can you provide the test code for your edge cases?

[josesimoes]

Well... there was no version information on the source files and I can't recall for how long we've been using it, I would guess it should be v1.1...

Maybe if you look at the PR that removed it, you can recognize the code... it's nanoframework/nf-interpreter#3020 (please look for Helpers/nanoprintf.c and .h

As for the test code, it may be a little tricky... this is being tested from C# which is using the .NET format specifiers, which at some point are being "translated" to printf before the call happens.

Calls happen mostly here @ DoPrintfOnDataType() which is called from Format_G() and other Format_nnn. G is the problematic one as it handles the doubles and floats. All the others look OK.

Test code (more values) which are failing come from
https://github.com/nanoframework/CoreLibrary/blob/ff784582b9c536e9e6fddeb84405763fb9f39455/Tests/NFUnitTestConversions/UnitTestConvertTests.cs#L225

Here's the output of the last failing one:
Expected:<1.7976931348623157E+308>. Actual:<1.797693134862316E+308>. double.MaxValue value failed

More details on the printf format strings being used in #2.
Hope this helps... and thank you again for looking into this.

[skirridsystems]

I had a look back through the history. You added es-printf on 16 May last year, but it was a modified version of my es-printf 1.3. I don't know where it came from as there are no forks of my repo and I've had no PRs, so I'm guessing it was you that modified it before adding?

The only significant change was to add specific tests for DBL_MAX and DBL_MIN, increase FLOAT_DIGITS by 1, and add buffer overrun protection.

That might explain why your specific tests for DBL_MAX/MIN were working better before. Maybe the FLOAT_DIGITS change was a fix for the rounding errors in integer output?

[skirridsystems]

Actually your test for DBL_MIN was incorrect. DBL_MIN is the smallest positive value that can be represented, which I believe is 2.2250738585072014e-308. Apart from fixing the rounding error, do these additional tests achieve anything apart from forcing the test case to pass at that specific value? Wouldn't the second largest representable number then fail the test instead?

[josesimoes]

The difference on the double min comes from .NET 😉 which uses this as the min value:
https://learn.microsoft.com/en-us/dotnet/api/system.double.minvalue?view=net-8.0

[skirridsystems]

That's the negative value of the double max, but negative values have already been converted to positive before that stage.

In C/C++ it means something different :-/
https://learn.microsoft.com/en-us/cpp/c-runtime-library/data-type-constants?view=msvc-170

[josesimoes]

Simon,

Thank for reviewing the code and point out those differences.
Now that I look more closely to this, I have a vague recollection of adding those changes. And yes, those checks were added to deal specifically with the boundary values of .NET double type. And no, all the other tests were passing.

I'm adding back es-printf in nanoframework/nf-interpreter#3026.

All our unit tests are 🟢
https://dev.azure.com/nanoframework/nf-interpreter/_build/results?buildId=49713&view=logs&j=60d69b93-63d4-5ecd-210f-9899d7535108&t=8b31b7c8-5aa3-558c-571e-f01bd2ab2143

[josesimoes]

Now that we're back to es-printg, appreciate your insight about the original issue asked here please. Do you know why we're seeing that output when printing integer values "as doubles" using that format string? Anyway to fix this as it's rather inconvinient?

Issue reported here nanoframework/Home#1484

[skirridsystems]

I understand your concern, but it's not a straightforward thing to fix. Converting double to human-readable accurately without losing any significant digits is really hard. If you look at dtoa.c in newlib-nano you'll see how it can be done.
https://github.com/32bitmicro/newlib-nano-1.0/blob/master/newlib/libc/stdlib/dtoa.c

But it's a lot of code and the primary use case for es-printf is to minimise code size on resource-constrained devices (think 8-bit micros) and getting 19 significant figures just isn't needed in most cases. I've gone for the simplest implementation which executes as fast as possible in the smallest code space, and loses a few bits in rounding errors. I'm guessing you mainly run on 32-bit devices with more than a 16kBytes of code space, so the resource issue is not as critical.

There are ways of making it pass the tests, e.g. by checking for integers and for DBL_MAX/DBL_MIN, but they don't really fix the problem, just hide it so it's still there on values you don't test.

So my suggestion is to use the implementation in newlib-nano instead of mine.

[josesimoes]

Simon,

Understood. But I really like your implementation! 😉
And yes, .NET nanoFramework is running on 32 bits MCUs. Still, our goal is to save as much as flash and RAM as possible, therefore the "quest" for better and smaller implementations.

So. besides adding back es-printf, I've made an improvement in the calling code to deal with the loss of significant digits with doubles. Considering that the reported issues only occurs with double values without decimal parts, I've added a pre-check for this situation and, when it's the case, reduce the number of significant digits to a value that's safe to use with es-printf. So far, couldn't find any side effects. All tests are green.

Thank you again for your help on this and, of course, for this handy library! 👍🏻