Partial replace call to snprintf for formating floatingpoint numbers. #7264

ghost · 2019-11-08T14:38:15Z

Because the description of this PR is long and contains diagrams, I put everything into printFloat.pdf.

TL;DR: Read only the abstract.

Still TL;DR: This only replaces the 'f' specifier for float and double. The algorithm is correct, in most cases faster than the old one (some optimizations are still WIP), it implements rounding according to IEEE754 (with a different tie than the old one), but no internationalisation, because that causes a lot of trouble and should be done elsewhere.

ghost · 2019-11-08T16:13:41Z

The failure on eBay/tsv-utils is due to the different tie in rounding. This is intentional (I think the new version is better) but might be discussed.

UplinkCoder

this has not gotten enough testing. Try double.max (1.0/3.0) inf -inf machine epsilon pi 2pi and a few other well known constants, at the very least.

p0nce · 2019-11-09T13:33:25Z

There are only 2^32 float so you could do an exhaustive check there. (I don't mean in a commited unittest)

ghost · 2019-11-09T15:35:04Z

There are only 2^32 float so you could do an exhaustive check there. (I don't mean in a commited unittest)

I did that allready with two different format strings - see pdf.

ghost · 2019-11-09T15:37:42Z

this has not gotten enough testing. Try double.max (1.0/3.0) inf -inf machine epsilon pi 2pi and a few other well known constants, at the very least.

inf (line 7743/7744) and epsilon (line 7788) are allready in the unittests. I'll add max and some special constants.

dlang-bot · 2019-11-10T09:17:45Z

Thanks for your pull request and interest in making D better, @berni44! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
I have provided a detailed rationale explaining my changes
New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✓	9889	normal	Incorrect rounding on floating value formatting
✗	20320	normal	format("%f") leeds to wrong output
✗	20371	normal	std.format limited to 500 characters for floats

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + phobos#7264"

ghost · 2019-11-10T09:18:47Z

added more unittests (and sorry @UplinkCoder I overlooked, that the eps-tests where float not double, added the double tests now)

ghost · 2019-11-17T13:34:33Z

While working on %a I discovered a bug in the implementation of the rule "Round to nearest, ties away from zero". It was actually "Round to nearest, ties up", which is different for negative numbers. I fixed this (and the wrong unittests) now.

p0nce · 2019-11-17T14:37:58Z

Just to say that this seems like fantastic work if the PDF says the truth. For localization I also think it fixes more undiscovered bugs than it breaks (there are more people unaware of localization using snprintf unknowingly, than people using D and relying on printf localization)

ghost · 2019-11-17T17:47:28Z

Just to say that this seems like fantastic work if the PDF says the truth.

Thanks for compliment! :-) If you want to check the content of the pdf: I put all needed functions in the repo and would be happy, if someone could verify the results...

ghost · 2019-12-04T16:58:30Z

Changed tie for rounding to the current behaviour. I'll address this later in a separate PR. I also added improvements suggested in PR #7285.

ghost · 2019-12-04T20:11:57Z

Does anyone has an idea, what this 32bit-windows compiler bug is about?

ghost · 2019-12-07T13:44:01Z

Merged everything and run the external tests (for %f and %a): all passed.

I changed bias from long to int, because I guess, this is the reason, why win32 crashed after the last change (had something similar resently in an other PR, as far as I remember). Let's see...

I also unified the tests for large precissions. I think there is no merit in having a string literal, which essentially consists of 1000 zeros in the source code. For checking the bug, it's sufficient to just check the length.

ghost · 2020-01-14T14:46:59Z

Fixed problem with global import of FloatingPointControl. See #7346 for more details.

ghost · 2020-02-07T13:48:03Z

Fixed the merge conflicts.

std/format.d

thewilsonator · 2020-02-09T04:12:08Z

std/format.d

            return printFloatA(val, f, rm, sgn, exp, mnt, is_upper);
        case 'e': case 'E':
            return printFloatE(val, f, rm, sgn, exp, mnt, is_upper);
+        case 'f': case 'F':
+            return printFloatF(val, f, rm, sgn, exp, mnt, is_upper);


Following the success of the float zero factorisation, I'm beginning to think that the best way to organise the printing is not by format specifier (%a,%f,%g,%e) but by "category" (for want of a better term), i.e.

inf/nan (handled above),

Algorithm A: For large exponents (exp >= T.mant_dig)

Algorithm B: For small exponents (exp < T.mant_dig - 61)

Algorithm C: For exponents close to 0. (already done)

Since there seems to be much more in common between the different ranges of numbers than there is between the different format specifiers.

Currently, only the format specifiers f and F are supported and only float and double.

…floats

…e enough

rainers · 2020-02-21T15:44:21Z

Rebased and adapted to not allocate as in #7393

thewilsonator · 2020-02-28T01:47:37Z

Implemented in another PR

rainers · 2020-02-28T07:31:08Z

Implemented in another PR

I'm not following phobos development closely, but which one would that be?

thewilsonator · 2020-02-28T07:32:02Z

#7393 No?

rainers · 2020-02-28T07:48:34Z

#7393 No?

#7285 and #7318 implemented "%a" and "%e" formatting, and #7393 removed some introduced allocations. This PR implements "%f" formatting. I only rebased it and modified it to not introduce allocations again.

ljmf00 · 2020-10-18T22:17:43Z

Do you anyone can push this forward @rainers or @thewilsonator ? This PR is crucial to have purity on format for floating point.

RazvanN7 · 2021-01-25T09:35:24Z

Closing this in favor of #7757

ghost marked this pull request as ready for review November 8, 2019 19:33

ghost requested a review from andralex as a code owner November 8, 2019 19:33

UplinkCoder requested changes Nov 9, 2019

View reviewed changes

ghost requested a review from UplinkCoder November 14, 2019 12:07

ghost mentioned this pull request Nov 17, 2019

Partial replace call to snprintf for '%a' and float or double. #7285

Merged

dlang-bot added the Severity:Bug Fix label Dec 7, 2019

This was referenced Dec 14, 2019

Partial replace call to snprintf for '%e' and float or double. #7318

Merged

Fix Issue 5628 - std.math unittest disabled - roundoff error in pow() #7321

Merged

ghost mentioned this pull request Jan 13, 2020

Merging stable back into master #7351

Merged

ghost mentioned this pull request Jan 28, 2020

Fix std.format FP printing for soft-float targets #7377

Merged

thewilsonator reviewed Feb 8, 2020

View reviewed changes

std/format.d Show resolved Hide resolved

ghost mentioned this pull request Feb 8, 2020

fix Issue 20566 - std.sformat should avoid allocating memory when pri… #7393

Merged

thewilsonator reviewed Feb 9, 2020

View reviewed changes

Bernhard Seckinger added 3 commits February 21, 2020 16:23

Partial replace call to snprintf for formating floatingpoint numbers.

9a4a41b

Currently, only the format specifiers f and F are supported and only float and double.

Fix partially Issue 20320 - format("%f") leeds to wrong output

535e473

Fix partially Issue 20371 - std.format limited to 500 characters for …

72a95d4

…floats

Bernhard Seckinger and others added 2 commits February 21, 2020 16:23

Fix Issue 9889 - Incorrect rounding on floating value formatting

4d30160

fix merge and adapt to #7393 to not allocate if passed buffer is larg…

53dd2f8

…e enough

thewilsonator closed this Feb 28, 2020

rainers reopened this Feb 28, 2020

timotheecour mentioned this pull request May 13, 2020

ryu timotheecour/Nim#108

Open

berni44 mentioned this pull request Jan 20, 2021

Partial replace call to snprintf for formating floatingpoint numbers for %f and %F #7757

Merged

RazvanN7 closed this Jan 25, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Partial replace call to snprintf for formating floatingpoint numbers. #7264

Partial replace call to snprintf for formating floatingpoint numbers. #7264

ghost commented Nov 8, 2019

ghost commented Nov 8, 2019

UplinkCoder left a comment

p0nce commented Nov 9, 2019 •

edited

Loading

ghost commented Nov 9, 2019

ghost commented Nov 9, 2019

dlang-bot commented Nov 10, 2019 •

edited

Loading

ghost commented Nov 10, 2019

ghost commented Nov 17, 2019

p0nce commented Nov 17, 2019

ghost commented Nov 17, 2019

ghost commented Dec 4, 2019

ghost commented Dec 4, 2019

ghost commented Dec 7, 2019

ghost commented Jan 14, 2020

ghost commented Feb 7, 2020

thewilsonator Feb 9, 2020

rainers commented Feb 21, 2020

thewilsonator commented Feb 28, 2020

rainers commented Feb 28, 2020

thewilsonator commented Feb 28, 2020

rainers commented Feb 28, 2020

ljmf00 commented Oct 18, 2020

RazvanN7 commented Jan 25, 2021

Partial replace call to snprintf for formating floatingpoint numbers. #7264

Partial replace call to snprintf for formating floatingpoint numbers. #7264

Conversation

ghost commented Nov 8, 2019

ghost commented Nov 8, 2019

UplinkCoder left a comment

Choose a reason for hiding this comment

p0nce commented Nov 9, 2019 • edited Loading

ghost commented Nov 9, 2019

ghost commented Nov 9, 2019

dlang-bot commented Nov 10, 2019 • edited Loading

Bugzilla references

Testing this PR locally

ghost commented Nov 10, 2019

ghost commented Nov 17, 2019

p0nce commented Nov 17, 2019

ghost commented Nov 17, 2019

ghost commented Dec 4, 2019

ghost commented Dec 4, 2019

ghost commented Dec 7, 2019

ghost commented Jan 14, 2020

ghost commented Feb 7, 2020

thewilsonator Feb 9, 2020

Choose a reason for hiding this comment

rainers commented Feb 21, 2020

thewilsonator commented Feb 28, 2020

rainers commented Feb 28, 2020

thewilsonator commented Feb 28, 2020

rainers commented Feb 28, 2020

ljmf00 commented Oct 18, 2020

RazvanN7 commented Jan 25, 2021

p0nce commented Nov 9, 2019 •

edited

Loading

dlang-bot commented Nov 10, 2019 •

edited

Loading