Broken tests with "legacy" sys.float_repr_style

[skirpichev]

Bug report

Bug description:

In #128005 broken (by downstream patch) build reveal some problems in the CPython test suite. Except for few explicitly marked tests, other don't care about different repr() behavior for floats. Many tests fail when interpreter built with _PY_SHORT_FLOAT_REPR set to 0: #128005 (comment)

Currently we don't test platforms with sys.float_repr_style == "legacy". Maybe we should? At least this define can be explicitly set by some configure option and we could test such build.

Then failed tests either should be fixed (by using floats, having exact decimal representation, like 1.25). Or just xfailed out with suitable decorator.

Is this worth?

BTW, as mentioned by Victor Stinner, all PEP 11 platforms support the floatting point "short" format.

CPython versions tested on:

CPython main branch

Operating systems tested on:

No response

Linked PRs

• gh-128051: fix tests if sys.float_repr_style is 'legacy' #135908
• [3.14] gh-128051: Fix tests if sys.float_repr_style is 'legacy' (GH-135908) #136025
• [3.13] gh-128051: Fix tests if sys.float_repr_style is 'legacy' (#135908) #136026

[vstinner]

If something is done, I suggest skipping failing tests if sys.float_repr_style is "legacy".

[skirpichev]

There are failing doctests too.

I think that in most cases it's possible to choose different values. If so, why not fix tests?

Do you think that there should be a real system with "legacy" repr or it's better just build CPython with -D_PY_SHORT_FLOAT_REPR=0 added to CFLAGS?

CC @pablogsal (buildbots)

[vstinner]

We don't test every possible config options in buildbots. For example, I don't think that we have a buildot treating warnings as errors, whereas it would be important to have one (IMO).

[pablogsal]

Do you think that there should be a real system with "legacy" repr or it's better just build CPython with -D_PY_SHORT_FLOAT_REPR=0 added to CFLAGS?

CC @pablogsal (buildbots)

Currently we don't have such buldbots unfortunately. Someone interested in testing in this configuration should add one following the procedure

[skirpichev]

Maybe just add a job in the GitHub CI, that will build CPython with -D_PY_SHORT_FLOAT_REPR=0?

I have no (alive) hardware around with legacy repr. Though - if we actually support this configuration - I think it's good to test it. E.g. this code is actually changed from time to time. Maybe this also help people to choose better floating-point values in tests (exact or having same repr in legacy/short modes).

Or... Maybe it's time just to drop 'legacy' repr? Are there alive users of such configuration? CC @mdickinson

Attached test log with broken tests (./python -m test), around ten of them. Attached patch fix most of tests, few disabled for 'legacy' repr. (Though, maybe I should instead disable more tests, where we can't/won't use some exactly representable decimal fractions.)

Let me know if this worth a PR.

legacy.log.txt

0001-gh-128051-fix-tests-if-sys.float_repr_style-is-legac.patch.txt

[mdickinson]

Or... Maybe it's time just to drop 'legacy' repr? Are there alive users of such configuration? CC @mdickinson

I'd be delighted to see the "legacy" mode finally die.

The tricky part is that the "short" repr code requires a way to set the FPU precision, and the mechanism for doing so isn't standardised, so without the legacy mode, getting Python to run on any new platform potentially requires figuring out the FPU precision puzzle for that platform.

FWIW, I think the "right" solution here is to ditch dtoa.c altogether in favour of something like Dragonbox (for the float-to-string direction) and ... um ... something else for string-to-float conversions. (That something else can't just be strtod, though, since we can't rely on that being correctly rounded.)

[mdickinson]

It's also worth noting that none of the recent crop of improved float-to-string converters (Ryu, Grisu, Dragonbox, ...) replaces even just the float -> string part of dtoa.c completely, since we still want things like this to work:

>>> import math
>>> f"{math.pi:.53f}"
'3.14159265358979311599796346854418516159057617187500000'

[skirpichev]

That something else can't just be strtod, though, since we can't rely on that being correctly rounded.

The C standard recommends it, if string has at most DBL_DECIMAL_DIG digits. Though, quick tests on Glibc show different timings (here PyOS_ascii_strtod is the current fallback code):

Benchmark	ref	PyOS_ascii_strtod	strtod
float('1.25')	306 ns	690 ns: 2.25x slower	561 ns: 1.83x slower
float('3.141592653589793')	1.06 us	867 ns: 1.22x faster	745 ns: 1.42x faster
float('1.11231e-10')	334 ns	852 ns: 2.55x slower	732 ns: 2.19x slower
float('-1.2981e+100')	931 ns	763 ns: 1.22x faster	645 ns: 1.44x faster
Geometric mean	(ref)	1.40x slower	1.18x slower

since we still want things like this to work:

>>> import math
>>> f"{math.pi:.53f}"
'3.14159265358979311599796346854418516159057617187500000'

This is something working with the printf() or I miss something?

$ cat a.c
#include <stdio.h>
int main()
{
    printf("%.53f\n", 3.141592653589793);
    return 0;
}
$ cc a.c && ./a.out
3.14159265358979311599796346854418516159057617187500000

[vstinner]

0001-gh-128051-fix-tests-if-sys.float_repr_style-is-legac.patch.txt

This change mostly LGTM. Please create a PR.

Attached patch fix most of tests

It's ok if we don't fix all tests. I don't think that we can say that legacy float repr is currently supported. We provide best effort support.