Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rounding errors in python #2236

Closed
fornwall opened this issue Mar 10, 2018 · 12 comments
Closed

Rounding errors in python #2236

fornwall opened this issue Mar 10, 2018 · 12 comments
Labels
bug report Something is not working properly help wanted Help is wanted in order to solve the issue

Comments

@fornwall
Copy link
Member

From @meeuw on March 2, 2018 20:50

When I convert a float to string I get weird rounding errors using Python 3.6.4:

$ python3
Python 3.6.4 (default, Jan  7 2018, 03:53:53)
[GCC 4.2.1 Compatible Android Clang 5.0.300080 ] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> str(0.1)
'0.10000000000000001'

According to the documentation it should display 0.1:
https://docs.python.org/3/tutorial/floatingpoint.html

I'm running termux on a Asus C101P Chromebook (ARMv8 processor).

Copied from original issue: termux/termux-app#604

@fornwall
Copy link
Member Author

From @iRomanyshyn on March 10, 2018 12:38

Oops, I made duplicate bug #617 with same problem.

@fornwall fornwall added bug report Something is not working properly help wanted Help is wanted in order to solve the issue labels Mar 10, 2018
@fornwall fornwall changed the title rounding errors in python Rounding errors in python Mar 10, 2018
@Neo-Oli
Copy link
Member

Neo-Oli commented Mar 12, 2018

Alright I googled a bit about this issue. It most probably comes from the fact that we use clang instead of gcc to compile python. I believe it all stems from this change: https://bugs.python.org/issue30124

Excerpt from the Makefile
# bpo-30104: dtoa.c uses union to cast double to unsigned long[2]. clang 4.0
# with -O2 or higher and strict aliasing miscompiles the ratio() function
# causing rounding issues. Compile dtoa.c using -fno-strict-aliasing on clang.
# https://bugs.llvm.org//show_bug.cgi?id=31928
Python/dtoa.o: Python/dtoa.c

According to this SO thread that class is responsible for the short float display
https://stackoverflow.com/questions/28493114/precision-of-reprf-strf-printf-when-f-is-float

Comment in Include/pyport.h:

/*  The functions _Py_dg_strtod and _Py_dg_dtoa in Python/dtoa.c (which are
 *  required to support the short float repr introduced in Python 3.1) require
 *  that the floating-point unit that's being used for arithmetic operations
 *  on C doubles is set to use 53-bit precision.  It also requires that the
 *  FPU rounding mode is round-half-to-even, but that's less often an issue.
 *
 *  If your FPU isn't already set to 53-bit precision/round-half-to-even, and
 *  you want to make use of _Py_dg_strtod and _Py_dg_dtoa, then you should
 *
 *     #define HAVE_PY_SET_53BIT_PRECISION 1
 *
 *  and also give appropriate definitions for the following three macros:
 *
 *    _PY_SET_53BIT_PRECISION_START : store original FPU settings, and
 *        set FPU to 53-bit precision/round-half-to-even
 *    _PY_SET_53BIT_PRECISION_END : restore original FPU settings
 *    _PY_SET_53BIT_PRECISION_HEADER : any variable declarations needed to
 *        use the two macros above.
 *
 * The macros are designed to be used within a single C function: see
 * Python/pystrtod.c for an example of their use.
 */

Note in the python bug report:

Note 1: We consider that Clang 4.0 is wrong, whereas GCC respects the C99 standard for aliasing on unions. But I don't think that it matters much who is wrong or not :-)

@Neo-Oli
Copy link
Member

Neo-Oli commented Mar 12, 2018

Although compiling with gcc doesn't seem to change anything, so I am probably wrong.
According to the python docs (https://docs.python.org/3/tutorial/floatingpoint.html)

Historically, the Python prompt and built-in repr() function would choose the one with 17 significant digits, 0.10000000000000001. Starting with Python 3.1, Python (on most systems) is now able to choose the shortest of these and simply display 0.1.

Now we just need to figure out why termux is not one of those "most systems".

Note: This isn't a bug in that it calculates wrong. Whether or not str(0.1) displays 0.1 or 0.10000000000000001, internally it is still 0.1000000000000000055511151231257827021181583404541015625

@tomty89
Copy link
Contributor

tomty89 commented Mar 12, 2018

Maybe it's a bionic vs glibc thing?

@Neo-Oli
Copy link
Member

Neo-Oli commented Mar 12, 2018

I asked the smart people on #python about it. Here's the conversation:

16:10 frumpylava: https://docs.python.org/3/tutorial/floatingpoint.html Says that "Starting with Python 3.1, Python (on most systems) is now able to choose the shortest of these and simply display 0.1.". What designates a system to be one of those "most system"? Because I have a system where that doesn't happen and I'd like to know why.
16:11 Wooble: frumpylava: what's the system? What's it doing instead?
16:12 frumpylava: Wooble: It's Android and it's displaying "0.10000000000000001" for "0.1".
16:12 papna: frumpylava: https://github.com/python/cpython/blob/9fb84157595a385f15799e5d0729c1e1b0ba9d38/Python/pystrtod.c
16:13 papna: frumpylava: Looks like the PY_NO_SHORT_FLOAT_REPR def is used to control it?
16:14 papna: frumpylava: I think it might just be whoever compiled your CPython, I dunno for sure.
16:15 frumpylava: papna: Yeah, I'm trying to fix that compilation.
16:17 Wooble: frumpylava: https://github.com/python/cpython/blob/13ff24582c99dfb439b1af7295b401415e7eb05b/Include/pyport.h#L458 looks like when compiling it thinks you don't have ieee-754 floats. No idea if that's true, though.
16:18 Yhg1s: frumpylava: shortest-unique-float support requires direct control of the FPU, which is something Python needs to be taught about for the specific target architecture.
16:18 Yhg1s: frumpylava: I highly recommend not worrying about this.
16:23 frumpylava: Yhg1s: So, how would I go about teaching python that? Is there an example somewhere for a different architecture?
16:24 Yhg1s: frumpylava: the relevant macros are in Include/pyport.h. They generally involve asm instructions. I don't know how you're building Python. Again, it would be much easier to not worry about whether you have the shortest canonical form.
16:26 frumpylava: Yhg1s: This is the build of python that I'm trying to fix https://github.com/termux/termux-packages/tree/master/packages/python
16:28 frumpylava: Yhg1s: It cross compiles for arm64 with clang.
16:29 Yhg1s: frumpylava: then you need a way to do 53-bit floating point arithmetic, and implement the Py_SET_53BIT_PRECISION* macros with that.
16:29 Yhg1s: frumpylava: or, as I said, stop worrying about having the shortest canonical form representation.
16:32 frumpylava: Yhg1s: This is too complicated for me, but I am going to copy this conversation into the issue about the problem. Maybe someone else will come up with a solution.
16:32 Yhg1s: frumpylava: well, for ~everyone the simple solution is to just not care about having the shortest representation of flotas.

@tomty89
Copy link
Contributor

tomty89 commented Mar 12, 2018

FWIW I am on aarch64 but python in Arch proot gives 0.1 when I type 0.1 or 1/10 (while python in Termux gives 0.10000000000000001 for both cases).

@Neo-Oli
Copy link
Member

Neo-Oli commented Mar 12, 2018

Maybe we can just patch out PY_NO_SHORT_FLOAT_REPR and hope it works? ¯\_(ツ)_/¯

@vivekjuneja
Copy link

Any plans of releasing this change ?

@its-pointless
Copy link
Contributor

i will have a look today

@its-pointless
Copy link
Contributor

i will make a pull request in a little bit.

@its-pointless
Copy link
Contributor

pr is tested on i686 should be fine.

@fornwall
Copy link
Member Author

fornwall commented Apr 3, 2018

Great work @its-pointless ! This should now be resolved in the latest package versions (3.6.5-1 for python, and 2.7.14-3 for python2).

yan12125 pushed a commit to yan12125/python3-android that referenced this issue Apr 5, 2018
@ghost ghost locked and limited conversation to collaborators Oct 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug report Something is not working properly help wanted Help is wanted in order to solve the issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants