Adding ' ' flag to string.format & double rounding method #3283
Conversation
|
I don't think that I like 3.1414999 rounding to 3.142 Note that I would expect 0.146 to round to 0.15 (for 2 dp) and to 0.1 (for 1 dp). |
|
Do you like rounding 3.1415 to 3.141 (for 3 dp)? The issue is that 3.1415 is represented as 3.141499996185303 thus "wrong" rounding. ="%.2f"%0.146
0.15
="%.1f"%0.146
0.1 |
|
If there isn't a representation for the value 3.1415, then you have to use the closest representation -- and since it is less than 3.141500000000, then it should round to 3.141. |
|
...which is not very intuitive especially when print(3.1415) prints 3.1415 (and hides the real representation). |
|
Your patches seem to have lines with trailing whitespace.
and this firmware was buiilt from it. With your patches, |
|
This code does weird stuff if the bottom 29 bits of the float are zero -- this will mean that printing increasing double values will not result in increasing numbers being displayed (in the double version of the build). These are the values printed by C for floats close to 3.1415 These values seem plausible. I would hope that the Lua code produces the same values...... |
|
Now 32bit Lua does what's in your printout. The only difference with this patch compared to your printout is in the 3.1415 line (%.3f): Mathematically it's wrong. Intuitively (if you don't care about how floats are stored) it's correct. |
|
I think that having different rules for lua32 than every other 32 bit float implementation is asking for trouble. |
|
Well this is very relevant comment. Though I still like this workaround. Would it be acceptable for everyone to have this an optionality configurable in |
vsky279
force-pushed
the
c_stringformat
branch
from
85f3df8 to
3a98c8a
Compare
vsky279
force-pushed
the
c_stringformat
branch
from
3a98c8a to
127a30e
Compare
|
I like the option but we should use another name, because what seems "intuitive" to whom, is more of a feeling and may change. There are many explicitly-named kinds of rounding in WP-en. I'll try to figure out which one is the one I learned in school math. Until then my draft option name would be Edit: In my tests so far, I forgot about negative numbers. Here's how my Ubuntu's bash's printf rounds: $ printf '%8.3f\t%8.4g\t%8.6g\n' {,-}{,12}3.1415{,,}
3.141 3.141 3.1415
123.142 123.1 123.142
-3.141 -3.141 -3.1415
-123.142 -123.1 -123.142I'll try and compare the LUAs soon. |
|
Most probably school math rounding is variant named "Commercial rounding" in German WP, or "Nearest integer rounding" in Russian WP. English WP tells about special case but not method itself. |
|
WP-en's section "Round half away from zero" has "This method, also known as commercial rounding, …" and that's what I currently consider intuitive. Would it be easily feasible to have an enum setting |
vsky279
force-pushed
the
c_stringformat
branch
from
127a30e to
9dd72d7
Compare
It seems that Ubuntu's printf suffers the same issue as 3.1415 rounds to 3.141 in your printout. Maybe it uses single-precision float? Thanks for your thoughts. In order not to complicate the code I suggest #define SPRINTF_ROUNDING_MODE_DOUBLEROUNDINGShould there be other rounding method implemented the directive will look like |
vsky279
changed the title
|
Are tostring/tonumber or sjson.{en,de}code affected by this? Should we test each rounding method for rounding error accumulation with repeated conversion, or are there some theoretical guarantees that make it redundant? |
|
It would be nice to have some tests that went to/from Json and verified
that it round tripped correctly
…On Sat, Sep 19, 2020, 11:38 M.K. ***@***.***> wrote:
Should we test each rounding method for rounding error accumulation with
repeated conversion tostring/tonumber or sjson.{en,de}code, or are there
some theoretical guarantees that make it redundant?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#3283 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AALQLTNSG5E7FRSFS27L5KLSGTF6JANCNFSM4RMBI4KQ>
.
|
|
I hope this test is what you've been asking for: a={}
for i=1,100 do a[i]=math.floor(node.random()*1e7)/(10^math.floor(math.random()*7)) end
b=sjson.encode(a)
c=sjson.decode(b)
for i,j in pairs(a) do
if a[i]~=c[i] then print(("mismatch [%d]: %.20f ~= %.20f, diff: %.20f"):format(i, a[i], c[i], a[i]-c[i])) end
endIt generates numbers with 7 significant digits with decimal point somewhere between these digits (%7g format is used when exporting to json). The test passes well in both cases ( Of course when 8 significant digits are generated, e.g. To wrap up: the rounding mode setting does not make any difference. It is activated only when less or equal than 6 dp are printed (condition |
|
Yes - that is exactly the right test.... Thank you for doing it. |
|
For debugging it might be nicer to use a fixed set of numbers. Maybe someone familiar with the bit-level side of the problem could (possibly later) replace the randomness with specifically chosen edge focus on the potential problems. |
|
Actually, I just filed #3287 which reveals that there are some values that do not round trip correctly. In fact, it turns out that around 60% of floats do not round trip exactly. This is ugly. |
|
Closed thanks to #3291. |
Fixes #3265.
devbranch rather than formaster.docs/*.This PR has two commits for the time being. These are to be squashed before merger.
The first commit implements ' ' flag to string format (%f, %g, %e)
It passes well the test performed by @mk-pmb in #3265 - this script.
Also it removes a part of code non compatible with Ubuntu's Lua 5.1 as well as Lua 5.3.
The code is removing the - sign for negative zero.
"%.3f"%-0.0004should indeed give-0.000.This test gives same results in NodeMCU Lua as well as in Ubuntu:
The second commit deals with intuitively wrong rounding
As discussed in #3265
"%.3f"%3.1415gives3.141(because of its float representation). Mathematically this is correct behavior and is not a bug per se while user would expect different behavior.I am proposing this workaround which I think is in line with the existing logic. Note that
=3.1415prints3.1415and not3.1414999. So when it is detected that such number would print this way the least significant bit of float mantise is added leading to "correct" rounding.This is the test it passes well
The result is the same as on Ubuntu Lua except for
%.20fof course as 64bit doubles are used.It works well also for
%eformat