search

aleaxit / gmpy

General Multi-Precision arithmetic for Python 2.6+/3+ (GMP, MPIR, MPFR, MPC)
https://gmpy2.readthedocs.io/en/latest/
GNU Lesser General Public License v3.0
510 stars 86 forks source link

test failures with mpfr 4.2.1 (change in MPFR formatted functions) #418

Closed doko42 closed 1 month ago

doko42 commented 1 year ago

gmpy2 sees test failures with mpfr 4.2.1, independent of the architecture, see for example

https://ci.debian.net/data/autopkgtest/testing/amd64/p/python-gmpy2/37232406/log.gz

mostly it's a missing "-", however there are a few tests in test_mpc.txt, which look suspicious with the double plus sign ...

Differences (ndiff with -expected +actual):

vinc17fr commented 1 year ago

I suppose that the bug is in gmpy2_format.c, function GMPy_MPC_Format, which currently has

    if (mpcstyle)
        strcat(tempbuf, " ");
    else {
        /* Need to insert + if imag is nan or +inf. */
        if (mpfr_nan_p(mpc_imagref(MPC(self))) ||
            (mpfr_inf_p(mpc_imagref(MPC(self))) &&
             mpfr_sgn(mpc_imagref(MPC(self))) > 0)) {
            strcat(tempbuf, "+");
        }

With the fix of the formatted output functions in GNU MPFR 4.2.1, the insertion of the + should no longer be needed.

Note also that the sign that appears before "nan" is indeterminate in general (just like in C), unless the sign bit is fixed by MPC.

vinc17fr commented 1 year ago

And to support both fixed and unfixed versions of MPFR, you could check whether the first character of imagbuf is a sign (+ or -).

But for the tests, note the indeterminate sign for "nan".

doko42 commented 1 year ago

`dpkg-buildpackage: info: source package python-gmpy2 dpkg-buildpackage: info: source version 2.1.2-2ubuntu1 dpkg-buildpackage: info: source distribution mantic dpkg-buildpackage: info: source changed by Matthias Klose doko@ubuntu.com dpkg-source --before-build . dpkg-buildpackage: info: host architecture amd64 debian/rules clean dh clean --buildsystem pybuild --with python3,sphinxdoc dh_auto_clean -O--buildsystem=pybuild I: pybuild base:291: python3.11 setup.py clean running clean removing '/home/packages/tmp/python-gmpy2-2.1.2/.pybuild/cpython3_3.11_gmpy2/build' (and everything under it) 'build/bdist.linux-x86_64' does not exist -- can't clean it 'build/scripts-3.11' does not exist -- can't clean it dh_autoreconf_clean -O--buildsystem=pybuild dh_clean -O--buildsystem=pybuild debian/rules binary dh binary --buildsystem pybuild --with python3,sphinxdoc dh_update_autotools_config -O--buildsystem=pybuild dh_autoreconf -O--buildsystem=pybuild dh_auto_configure -O--buildsystem=pybuild I: pybuild base:291: python3.11 setup.py config running config debian/rules override_dh_auto_build make[1]: Entering directory '/home/packages/tmp/python-gmpy2-2.1.2' dh_auto_build I: pybuild base:291: /usr/bin/python3 setup.py build running build running build_py creating /home/packages/tmp/python-gmpy2-2.1.2/.pybuild/cpython3_3.11_gmpy2/build/gmpy2 copying gmpy2/init.py -> /home/packages/tmp/python-gmpy2-2.1.2/.pybuild/cpython3_3.11_gmpy2/build/gmpy2 running egg_info creating gmpy2.egg-info writing gmpy2.egg-info/PKG-INFO writing dependency_links to gmpy2.egg-info/dependency_links.txt writing top-level names to gmpy2.egg-info/top_level.txt writing manifest file 'gmpy2.egg-info/SOURCES.txt' reading manifest file 'gmpy2.egg-info/SOURCES.txt' reading manifest template 'MANIFEST.in' adding license file 'COPYING' adding license file 'COPYING.LESSER' writing manifest file 'gmpy2.egg-info/SOURCES.txt' copying gmpy2/init.pxd -> /home/packages/tmp/python-gmpy2-2.1.2/.pybuild/cpython3_3.11_gmpy2/build/gmpy2 copying gmpy2/gmpy2.h -> /home/packages/tmp/python-gmpy2-2.1.2/.pybuild/cpython3_3.11_gmpy2/build/gmpy2 copying gmpy2/gmpy2.pxd -> /home/packages/tmp/python-gmpy2-2.1.2/.pybuild/cpython3_3.11_gmpy2/build/gmpy2 running build_ext building 'gmpy2.gmpy2' extension creating build creating build/temp.linux-x86_64-cpython-311 creating build/temp.linux-x86_64-cpython-311/src x86_64-linux-gnu-gcc -Wsign-compare -DNDEBUG -g -fwrapv -O2 -Wall -g -fstack-protector-strong -Wformat -Werror=format-security -g -fwrapv -O2 -g -O2 -ffile-prefix-map=/home/packages/tmp/python-gmpy2-2.1.2=. -flto=auto -ffat-lto-objects -fstack-protector-strong -fstack-clash-protection -Wformat -Werror=format-security -fcf-protection -fdebug-prefix-map=/home/packages/tmp/python-gmpy2-2.1.2=/usr/src/python-gmpy2-2.1.2-2ubuntu1 -Wdate-time -D_FORTIFY_SOURCE=2 -fPIC -I./src -I/usr/include/python3.11 -c src/gmpy2.c -o build/temp.linux-x86_64-cpython-311/src/gmpy2.o -DSHARED=1 x86_64-linux-gnu-gcc -shared -Wl,-O1 -Wl,-Bsymbolic-functions -Wl,-Bsymbolic-functions -g -fwrapv -O2 -Wl,-Bsymbolic-functions -flto=auto -ffat-lto-objects -Wl,-z,relro -Wl,-z,now -g -O2 -ffile-prefix-map=/home/packages/tmp/python-gmpy2-2.1.2=. -flto=auto -ffat-lto-objects -fstack-protector-strong -fstack-clash-protection -Wformat -Werror=format-security -fcf-protection -fdebug-prefix-map=/home/packages/tmp/python-gmpy2-2.1.2=/usr/src/python-gmpy2-2.1.2-2ubuntu1 -Wdate-time -D_FORTIFY_SOURCE=2 build/temp.linux-x86_64-cpython-311/src/gmpy2.o -L/usr/lib/x86_64-linux-gnu -lmpc -lmpfr -lgmp -o /home/packages/tmp/python-gmpy2-2.1.2/.pybuild/cpython3_3.11_gmpy2/build/gmpy2/gmpy2.cpython-311-x86_64-linux-gnu.so make man html -C /home/packages/tmp/python-gmpy2-2.1.2/docs SPHINXOPTS="-D today=\"1693742772\"" make[2]: Entering directory '/home/packages/tmp/python-gmpy2-2.1.2/docs' if [ ! -d _static ]; then mkdir _static; fi sphinx-build -b man -d _build/doctrees -D today="1693742772" . _build/man Running Sphinx v5.3.0 making output directory... done building [mo]: targets for 0 po files that are out of date building [man]: all manpages updating environment: [new config] 12 added, 0 changed, 0 removed reading sources... [ 8%] advmpz reading sources... [ 16%] conversion reading sources... [ 25%] cython reading sources... [ 33%] history reading sources... [ 41%] index reading sources... [ 50%] intro reading sources... [ 58%] mpc reading sources... [ 66%] mpfr reading sources... [ 75%] mpq reading sources... [ 83%] mpz reading sources... [ 91%] overview reading sources... [100%] win_build

looking for now-outdated files... none found pickling environment... done checking consistency... /home/packages/tmp/python-gmpy2-2.1.2/docs/win_build.rst: WARNING: document isn't included in any toctree done writing... gmpy2.3 { intro overview mpz advmpz mpq mpfr mpc cython conversion history } done build succeeded, 1 warning.

The manual pages are in _build/man.

Build finished. The manual pages are in _build/man. sphinx-build -b html -d _build/doctrees -D today="1693742772" . _build/html Running Sphinx v5.3.0 making output directory... done loading pickled environment... done building [mo]: targets for 0 po files that are out of date building [html]: targets for 12 source files that are out of date updating environment: 0 added, 0 changed, 0 removed looking for now-outdated files... none found preparing documents... done writing output... [ 8%] advmpz writing output... [ 16%] conversion writing output... [ 25%] cython writing output... [ 33%] history writing output... [ 41%] index writing output... [ 50%] intro writing output... [ 58%] mpc writing output... [ 66%] mpfr writing output... [ 75%] mpq writing output... [ 83%] mpz writing output... [ 91%] overview writing output... [100%] win_build

generating indices... genindex done writing additional pages... search done copying static files... done copying extra files... done dumping search index in English (code: en)... done dumping object inventory... done build succeeded.

The HTML pages are in _build/html.

Build finished. The HTML pages are in _build/html. make[2]: Leaving directory '/home/packages/tmp/python-gmpy2-2.1.2/docs' make[1]: Leaving directory '/home/packages/tmp/python-gmpy2-2.1.2' debian/rules override_dh_auto_test make[1]: Entering directory '/home/packages/tmp/python-gmpy2-2.1.2' dh_auto_test -- --system=custom --test-args="{interpreter} {dir}/test/runtests.py" I: pybuild base:291: python3.11 /home/packages/tmp/python-gmpy2-2.1.2/test/runtests.py

Unit tests for gmpy2 2.1.2 with Python 3.11.5 Mutliple-precision library: GMP 6.3.0 Floating-point library: MPFR 4.2.1 Complex library: MPC 1.3.1 Caching Values: (Cache size) 100 Caching Values: (Size in limbs) 128

Results for: test_context Attempted: 38 Failed: 0 Results for: test_gmpy2_abs Attempted: 83 Failed: 0 Results for: test_gmpy2_add Attempted: 82 Failed: 0 Results for: test_gmpy2_binary Attempted: 106 Failed: 0 Results for: test_gmpy2_cmp Attempted: 41 Failed: 0 Results for: test_gmpy2_comp Attempted: 89 Failed: 0 Results for: test_gmpy2_const Attempted: 33 Failed: 0 Results for: test_gmpy2_constructors Attempted: 34 Failed: 0 Results for: test_gmpy2_convert Attempted: 50 Failed: 0 Results for: test_gmpy2_divmod Attempted: 42 Failed: 0 Results for: test_gmpy2_floordiv Attempted: 72 Failed: 0 Results for: test_gmpy2_format Attempted: 102 Failed: 0 Results for: test_gmpy2_fused Attempted: 18 Failed: 0 Results for: test_gmpy2_lucas Attempted: 23 Failed: 0

File "/home/packages/tmp/python-gmpy2-2.1.2/test/test_gmpy2_math.txt", line 35, in test_gmpy2_math.txt Failed example: ctx.sqrt(mpfr(-380.25)) Differences (ndiff with -expected +actual):

Running external test programs. Running test_pack.py successful Running test_mpz_args.py successful E: pybuild pybuild:395: test: plugin custom failed with: exit code=1: python3.11 /home/packages/tmp/python-gmpy2-2.1.2/test/runtests.py dh_auto_test: error: pybuild --test -i python{version} -p 3.11 --system=custom "--test-args={interpreter} {dir}/test/runtests.py" returned exit code 13 make[1]: [debian/rules:19: override_dh_auto_test] Error 25 make[1]: Leaving directory '/home/packages/tmp/python-gmpy2-2.1.2' make: [debian/rules:12: binary] Error 2 dpkg-buildpackage: error: debian/rules binary subprocess returned exit status 2`

doko42 commented 1 year ago

the proposed patch fixes the double '++', there are unexpected results regarding nan/-nan

martingkelly commented 1 year ago

This was found recently in Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1050797. Looks like the patch from @doko42 should address it.

If the patch looks good, please let me know if you plan to release a bugfix version soon; if not, we can include this in packaging to fix this up for Debian until the next gmpy release.

doko42 commented 1 year ago

there is no patch from me, the nan/+nan issues are still present

vinc17fr commented 1 year ago

In addition to the correction I mentioned above, which fixes the double "+" sign, you should ignore the sign bit of nan since it is not used in these tests. I think that the easiest solution would be to check whether the real or imaginary part of the result is nan, and force the sign bit to positive before converting the value to a string. Perhaps I could provide a patch later.

vinc17fr commented 1 year ago

I think that the easiest solution would be to check whether the real or imaginary part of the result is nan, and force the sign bit to positive before converting the value to a string.

Actually this is not possible because the conversion to a string is the gmpy code, which is correct with #422. So, concerning the sign before nan differences, these are the only tests that should be modified. A solution could be to postprocess the result file to replace - by + before nanj (this is currently useless, but failures might occur in the future if this is not done) and remove the - before nan, then check the differences.

doko42 commented 1 year ago

please reopen. the patch in #422 only fixes the issue with the double plus sign

casevh commented 1 year ago

My apologies for the confusion. The test suite for the next version 2.2.0a1 passes on my machine. When I try one of the failures listed above, I believe I am getting the expected result.

gmpy2.mpfr_version() 'MPFR 4.2.1' gmpy2.mpc_version() 'MPC 1.3.1' nan = gmpy2.nan(); nan mpfr('nan')

I will test with gmpy2 2.1.5 tomorrow and try to reproduce the issue.

I am trying to release gmpy2 2.2.0 as soon as I can since it required for Python 3.12. But it already doesn't work with 3.13(dev).

vinc17fr commented 1 year ago

MPFR sets a positive sign by default to initialized variables (mpfr_init2), but if a variable is reused, it is possible that gmpy2.nan() yields a NaN with a negative sign, which could explain the failure. I don't know what gmpy2 does, though. In any case, the sign of NaN should be ignored in the tests.

skirpichev commented 1 year ago

the proposed patch fixes the double '++', there are unexpected results regarding nan/-nan

@doko42, your message shows test failures on the gmpy2 test suite. But it pass in https://github.com/aleaxit/gmpy/pull/422.

Did you test the patched version?

doko42 commented 1 year ago

no, just this patch on top of the 2.1.5 release. are there any patches in the trunk to get 2.1.5 working with mpfr 4.2.1?

skirpichev commented 1 year ago

no, just this patch on top of the 2.1.5 release

From https://github.com/aleaxit/gmpy/pull/422, correct? Then, there should be no test failures with our tests, e.g.: https://github.com/aleaxit/gmpy/actions/runs/6061180143/job/16446076083 (The "Build Wheels" workflow uses MPFR 4.2.1)

doko42 commented 1 year ago

yes, from #422

doko42 commented 1 year ago

my build logs: https://launchpad.net/ubuntu/+source/python-gmpy2/2.1.5-1ubuntu1

doko42 commented 1 year ago

I see my builds use mpclib 1.3.1, your's are using 1.2.1

skirpichev commented 1 year ago

Yeah, I'm trying to do an update: https://github.com/skirpichev/gmpy/actions/runs/6083541455

doko42 commented 1 year ago

also gmp 6.3.0 vs. 6.2.1

skirpichev commented 1 year ago

No. MPC version is irrelevant: https://github.com/skirpichev/gmpy/actions/runs/6083541455/job/16503615537 (test failure on macos-12 is not due to the test suite)

skirpichev commented 1 year ago

@doko42, the problem is that you test not the latest version, but the stable gmpy2 release(s), which is(are) ~250+ commits behind the current master.

The MPC and GMP versions are irrelevant: https://github.com/aleaxit/gmpy/pull/425

doko42 commented 1 year ago

yes, that's what I said in https://github.com/aleaxit/gmpy/issues/418#issuecomment-1706210390

also asking in https://github.com/aleaxit/gmpy/issues/418#issuecomment-1706202396 for relevant patches

doko42 commented 1 year ago

https://launchpad.net/~doko/+archive/ubuntu/ppa/+sourcepub/15141484/+listing-archive-extra for a trunk build. already stops in the doctests

skirpichev commented 1 year ago

On Tue, Sep 05, 2023 at 05:44:29AM -0700, Matthias Klose wrote:

[1]https://launchpad.net/~doko/+archive/ubuntu/ppa/+sourcepub/15141484/+listing-archive-extra for a trunk build. already stops in the doctests

You should use test/runtests.py as before to run tests. The test suite only partially was ported to the pytest framework, see #420.

doko42 commented 1 year ago

https://launchpad.net/~doko/+archive/ubuntu/ppa/+sourcepub/15141533/+listing-archive-extra

that seems to work, although I patched out the call to pytest at the end of the script, which causes a failure code. just running the pytest on it's own

anyway, please could you point out the relevant change which could be used for backporting?

skirpichev commented 1 year ago

On Tue, Sep 05, 2023 at 06:22:48AM -0700, Matthias Klose wrote:

[1]https://launchpad.net/~doko/+archive/ubuntu/ppa/+sourcepub/15141533/+listing-archive-extra

that seems to work, although I patched out the call to pytest at the end of the script, which causes a failure code.

What kind of failure?

(BTW, as you run sphinx-build without the gmpy2 package being installed, probably you shold set PYTHONPATH to ".")

anyway, please could you point out the relevant change which could be used for backporting?

Hmm, git bisect shows it's 7351e2e, probably it's the change in GMPy_MPFR_New().

skirpichev commented 1 year ago

Ok, after some cleanup of the https://github.com/aleaxit/gmpy/commit/7351e2eb1abf4b37a47a822eb3f3f29f90c7f854, I got this: gmpy2_cache.c.diff.txt

With this additional patch - the test suite pass for gmpy2-2.1.5: log.txt

doko42 commented 1 year ago

thanks, that looks good: https://launchpad.net/ubuntu/+source/python-gmpy2/2.1.5-1ubuntu2

What kind of failure? the script doesn't show any error output ;) the Debian packaging doesn't build in the srcdir, but in a separate dir. probably worth to set this or pass via an argument

skirpichev commented 1 year ago

On Tue, Sep 05, 2023 at 07:40:49AM -0700, Matthias Klose wrote:

thanks, that looks good: [1]https://launchpad.net/ubuntu/+source/python-gmpy2/2.1.5-1ubuntu2

Great. @casevh, probably we can close this issue as fixed in the master, unless you want to do a bugfix release, which will include this stuff.

 What kind of failure?
 the script doesn't show any error output ;)

Hmm. Could you point me to a relevant build log?

Here is an example, where the pytest package wasn't installed: (debian-test) @.**:~/src/gmpy $ rm build/ -rf && \ CFLAGS=-I/usr/local/include/ LDFLAGS=-L/usr/local/lib/ pip install . --verbose && \ LD_LIBRARY_PATH=/usr/local/lib/ python test/runtests.py [...] Results for: test_mpz_io Attempted: 60 Failed: 0

                          Summary - Attempted: 3303   Failed:    0

Running external test programs. /home/sk/src/gmpy/debian-test/bin/python: No module named pytest Running pytest failed

So, I would guess you did redirection of the stderr somewhere.

casevh commented 11 months ago

I have published release 2.2.0a1 on PyPi. Unfortunately, I forgot to include the source distribution. For reference, the source code is available at:

https://github.com/aleaxit/gmpy/archive/refs/tags/gmpy2-2.2.0a1.tar.gz

I will publish a2 soon.

casevh commented 11 months ago

Source for 2.2.0a1 has been uploaded.

skirpichev commented 2 months ago

@casevh, probably this can be closed as v2.2 released.