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

upgrade to flint 2.6.3 #29719

Closed
videlec opened this issue May 20, 2020 · 176 comments
Closed

upgrade to flint 2.6.3 #29719

videlec opened this issue May 20, 2020 · 176 comments

Comments

@videlec
Copy link
Contributor

videlec commented May 20, 2020

Flint 2.6.3 has been released

tarball: see checksums.ini [upstream_url]

Depends on #30805

Upstream: Reported upstream. Developers acknowledge bug.

CC: @timokau @kedlaya @jbalakrishnan @saraedum @isuruf @mkoeppe @kliem @videlec @w-bruns @slel

Component: packages: standard

Keywords: upgrade

Author: Dima Pasechnik, Matthias Koeppe

Branch/Commit: 7681b1a

Reviewer: Matthias Koeppe, Dima Pasechnik

Issue created by migration from https://trac.sagemath.org/ticket/29719

@videlec videlec added this to the sage-9.2 milestone May 20, 2020
@dimpase

This comment has been minimized.

@dimpase
Copy link
Member

dimpase commented May 21, 2020

Branch: public/packages/flint260

@dimpase
Copy link
Member

dimpase commented May 21, 2020

Commit: 3bb2c70

@dimpase
Copy link
Member

dimpase commented May 21, 2020

comment:1

this is from the latest alpha2 (commit a960857c7d8e5ea7c4d4c2958e38ec52778d85d9)

Patches still need to be sorted out.


New commits:

3bb2c70update flint to 2.6.0 (alpha version)

@dimpase
Copy link
Member

dimpase commented May 21, 2020

Author: Dima Pasechnik

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 21, 2020

Changed commit from 3bb2c70 to 9b6e67c

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 21, 2020

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

9b6e67cupdate flint to 2.6.0 (alpha version), remove patches

@dimpase

This comment has been minimized.

@dimpase
Copy link
Member

dimpase commented May 21, 2020

comment:3

OK, can be tested now. All our patches got upstreamed, so they are gone now.

@dimpase
Copy link
Member

dimpase commented May 21, 2020

comment:4

Neither arb 2.16 nor 2.17 work with this version of Flint.

One gets

gcc -fPIC -ansi -pedantic -Wall -O2 -funroll-loops -g -mpopcnt  -I/home/scratch2/dimpase/sage/sage/local/var/tmp/sage/build/arb-2.17.0/src -I/home/scratch2/dimpase/sage/sage/local/include -I/home/scratch2/dimpase/sage/sage/local/include -I/home/scratch2/dimpase/sage/sage/local/include -c mul_mpn.c -o ../build/fmpr/mul_mpn.lo -MMD -MP -MF "../build/fmpr/mul_mpn.d" -MT "../build/fmpr/mul_mpn.d" -MT "../build/fmpr/mul_mpn.lo"
In file included from ../fmpr.h:26,
                 from get_si.c:12:
../fmpz_extras.h:47:1: error: redefinition of 'fmpz_add_si'
   47 | fmpz_add_si(fmpz_t z, const fmpz_t x, slong y)
      | ^~~~~~~~~~~
In file included from ../fmpr.h:23,
                 from get_si.c:12:
../../../../../../../include/flint/fmpz.h:479:18: note: previous definition of 'fmpz_add_si' was here
  479 | FMPZ_INLINE void fmpz_add_si(fmpz_t f, const fmpz_t g, slong x)
      |                  ^~~~~~~~~~~
In file included from ../fmpr.h:26,
                 from get_si.c:12:
../fmpz_extras.h:56:1: error: redefinition of 'fmpz_sub_si'
   56 | fmpz_sub_si(fmpz_t z, const fmpz_t x, slong y)
      | ^~~~~~~~~~~
In file included from ../fmpr.h:23,
                 from get_si.c:12:
../../../../../../../include/flint/fmpz.h:487:18: note: previous definition of 'fmpz_sub_si' was here
  487 | FMPZ_INLINE void fmpz_sub_si(fmpz_t f, const fmpz_t g, slong x)
      |                  ^~~~~~~~~~~
make[5]: *** [../Makefile.subdirs:60: ../build/fmpr/get_si.lo] Error 1

@dimpase
Copy link
Member

dimpase commented May 21, 2020

comment:5

One apparently may use arb patch: https://github.com/fredrik-johansson/arb/commit/d3d9983231e0f034e86a1e75761627eb8213b704.patch - trying this with arb 2.17.0 now.

@fredrik-johansson
Copy link
Contributor

comment:6

Yes, there will be an immediate new Arb release. The current git master ought to work.

@dimpase
Copy link
Member

dimpase commented May 21, 2020

comment:8

Replying to @fredrik-johansson:

Yes, there will be an immediate new Arb release. The current git master ought to work.

there are doctest errors (I made https://github.com/dimpase/arb/releases/tag/2.17.1, and removed all the arb patches)

sage -t --warn-long 59.4 src/sage/rings/complex_arb.pyx
**********************************************************************
File "src/sage/rings/complex_arb.pyx", line 4554, in sage.rings.complex_arb.ComplexBall.elliptic_pi
Failed example:
    CBF(2,3).elliptic_pi(CBF(1,1))
Expected:
    [0.27029997361983 +/- ...e-15] + [0.715676058329095 +/- ...e-16]*I
Got:
    [0.2702999736198 +/- 4.51e-14] + [0.7156760583291 +/- 1.78e-14]*I
**********************************************************************
File "src/sage/rings/complex_arb.pyx", line 4662, in sage.rings.complex_arb.ComplexBall.elliptic_pi_inc
Failed example:
    n.elliptic_pi_inc(CBF.pi()/2, m)
Expected:
    [0.8934793755173 +/- ...e-14] + [0.95707868710750 +/- ...e-15]*I
Got:
    nan + nan*I
**********************************************************************
File "src/sage/rings/complex_arb.pyx", line 4664, in sage.rings.complex_arb.ComplexBall.elliptic_pi_inc
Failed example:
    n.elliptic_pi(m)
Expected:
    [0.89347937551733 +/- ...e-15] + [0.95707868710750 +/- ...e-15]*I
Got:
    [0.8934793755173 +/- 4.27e-14] + [0.9570786871075 +/- 9.91e-15]*I
**********************************************************************
File "src/sage/rings/complex_arb.pyx", line 4669, in sage.rings.complex_arb.ComplexBall.elliptic_pi_inc
Failed example:
    n.elliptic_pi_inc(CBF.pi()/2, m)
Expected:
    [0.2969588746419 +/- ...e-14] + [1.3188795332738 +/- ...e-14]*I
Got:
    nan + nan*I
**********************************************************************
File "src/sage/rings/complex_arb.pyx", line 4671, in sage.rings.complex_arb.ComplexBall.elliptic_pi_inc
Failed example:
    n.elliptic_pi(m)
Expected:
    [0.29695887464189 +/- ...e-15] + [1.31887953327376 +/- ...e-15]*I
Got:
    [0.2969588746419 +/- 3.79e-14] + [1.3188795332738 +/- 7.19e-14]*I
**********************************************************************
File "src/sage/rings/complex_arb.pyx", line 4747, in sage.rings.complex_arb.ComplexBall.elliptic_rj
Failed example:
    CBF(0,1).elliptic_rj(CBF(-1/2,1), CBF(-1,-1), CBF(2))
Expected:
    [1.004386756285733 +/- ...e-16] + [-0.2451626834391645 +/- ...e-17]*I
Got:
    [1.00438675628573 +/- 6.85e-15] + [-0.24516268343916 +/- 8.11e-15]*I
**********************************************************************
3 items had failures:
   1 of   2 in sage.rings.complex_arb.ComplexBall.elliptic_pi
   4 of  10 in sage.rings.complex_arb.ComplexBall.elliptic_pi_inc
   1 of   2 in sage.rings.complex_arb.ComplexBall.elliptic_rj
    [639 tests, 6 failures, 8.14 s]

and

sage -t --warn-long 59.4 src/sage/rings/real_arb.pyx
**********************************************************************
File "src/sage/rings/real_arb.pyx", line 295, in sage.rings.real_arb.arb_to_mpfi
Failed example:
    RIF(RBF(2)**(2**100)) # indirect doctest
Expected:
    Traceback (most recent call last):
    ...
    ArithmeticError: Error converting arb to mpfi. Overflow?
Got:
    [5.8756537891115869e1388255822130839282 .. +infinity]
**********************************************************************
File "src/sage/rings/real_arb.pyx", line 1660, in sage.rings.real_arb.RealBall.mid
Failed example:
    b.mid()
Expected:
    Traceback (most recent call last):
    ...
    RuntimeError: unable to convert to MPFR (exponent out of range?)
Got:
    +infinity
**********************************************************************
2 items had failures:
   1 of   7 in sage.rings.real_arb.RealBall.mid
   1 of   2 in sage.rings.real_arb.arb_to_mpfi
    [542 tests, 2 failures, 0.64 s]

@dimpase
Copy link
Member

dimpase commented May 22, 2020

comment:9

OK, I ended up using arb patch: ​https://github.com/fredrik-johansson/arb/commit/d3d9983231e0f034e86a1e75761627eb8213b704.patch
with arb 2.16.0 (the arb version in Sage now), and get one flint-related regression (a segfault)

sage: R.<x> = QQ['x']
sage: A,f = monsky_washnitzer.matrix_of_frobenius_hyperelliptic(x^5 - 2*x + 3, 5, 3)
-------------------------------------------------------------------------------------
/home/scratch2/dimpase/sage/sage/local/lib64/python3.7/site-packages/cysignals/signals.cpython-37m-x86_64-linux-gnu.so(+0x84a4)[0x7f98890224a4]
/home/scratch2/dimpase/sage/sage/local/lib64/python3.7/site-packages/cysignals/signals.cpython-37m-x86_64-linux-gnu.so(+0x8679)[0x7f9889022679]
/home/scratch2/dimpase/sage/sage/local/lib64/python3.7/site-packages/cysignals/signals.cpython-37m-x86_64-linux-gnu.so(+0xae41)[0x7f9889024e41]
/lib64/libc.so.6(+0x37ec0)[0x7f988a679ec0]
/lib64/libc.so.6(gsignal+0x145)[0x7f988a679e35]
/lib64/libc.so.6(abort+0x127)[0x7f988a664895]
/home/scratch2/dimpase/sage/sage/local/lib/libflint.so.14(+0x85dcd)[0x7f9880a64dcd]
/home/scratch2/dimpase/sage/sage/local/lib/libflint.so.14(flint_throw+0xe0)[0x7f9880a64eb0]
/home/scratch2/dimpase/sage/sage/local/lib/libflint.so.14(_nmod_poly_rem_q1+0xa1)[0x7f9880bd8bd1]
/home/scratch2/dimpase/sage/sage/local/lib/libflint.so.14(_nmod_poly_rem+0x195)[0x7f9880bc39f5]
/home/scratch2/dimpase/sage/sage/local/lib/libflint.so.14(_nmod_poly_gcd_euclidean+0x60)[0x7f9880babd50]
/home/scratch2/dimpase/sage/sage/local/lib/libflint.so.14(nmod_poly_gcd+0x95)[0x7f9880bb28b5]
/home/scratch2/dimpase/sage/sage/local/lib64/python3.7/site-packages/sage/rings/polynomial/polynomial_zmod_flint.cpython-37m-x86_64-linux-gnu.so(+0x1201c)[0x7f983d7b401c]
/lib64/libpython3.7m.so.1.0(_PyMethodDef_RawFastCallDict+0x33a)[0x7f988a402bea]
/lib64/libpython3.7m.so.1.0(+0x1ccd3f)[0x7f988a4c1d3f]
/home/scratch2/dimpase/sage/sage/local/lib64/python3.7/site-packages/sage/structure/element.cpython-37m-x86_64-linux-gnu.so(+0x134fc)[0x7f98897424fc]
/home/scratch2/dimpase/sage/sage/local/lib64/python3.7/site-packages/sage/structure/element.cpython-37m-x86_64-linux-gnu.so(+0x338a4)[0x7f98897628a4]
/lib64/libpython3.7m.so.1.0(_PyObject_FastCallKeywords+0x4bc)[0x7f988a433eec]
/lib64/libpython3.7m.so.1.0(+0x1404e9)[0x7f988a4354e9]
/lib64/libpython3.7m.so.1.0(_PyEval_EvalFrameDefault+0x55da)[0x7f988a46fdea]
/lib64/libpython3.7m.so.1.0(_PyEval_EvalCodeWithName+0x2f0)[0x7f988a4227e0]
/lib64/libpython3.7m.so.1.0(_PyFunction_FastCallKeywords+0x2a2)[0x7f988a423822]
/lib64/libpython3.7m.so.1.0(+0x14035f)[0x7f988a43535f]
...

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 22, 2020

Changed commit from 9b6e67c to d943e59

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 22, 2020

Branch pushed to git repo; I updated commit sha1. New commits:

d943e59backport flint compat patch from arb's trunk

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 22, 2020

Branch pushed to git repo; I updated commit sha1. New commits:

f80e315url in checksum, for convenience (to be changed)

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 22, 2020

Changed commit from d943e59 to f80e315

@dimpase
Copy link
Member

dimpase commented May 22, 2020

comment:12

experiments tell that precision needs to be at least 25 for this example to run, i.e. this works:

sage: monsky_washnitzer.matrix_of_frobenius_hyperelliptic(x^5 - 2*x + 3, 5, 25)

@dimpase
Copy link
Member

dimpase commented May 22, 2020

comment:13

But this works:

sage: R.<x> = QQ['x']
sage: C=HyperellipticCurve(x^5 - 2*x + 3)
sage: A,f=monsky_washnitzer.matrix_of_frobenius_hyperelliptic(C,5,3)
sage: A
[            4*5 + O(5^3)       5 + 2*5^2 + O(5^3) 2 + 3*5 + 2*5^2 + O(5^3)     2 + 5 + 5^2 + O(5^3)]
[      3*5 + 5^2 + O(5^3)             3*5 + O(5^3)             4*5 + O(5^3)         2 + 5^2 + O(5^3)]
[    4*5 + 4*5^2 + O(5^3)     3*5 + 2*5^2 + O(5^3)       5 + 3*5^2 + O(5^3)     2*5 + 2*5^2 + O(5^3)]
[            5^2 + O(5^3)       5 + 4*5^2 + O(5^3)     4*5 + 3*5^2 + O(5^3)             2*5 + O(5^3)]

So, apparently, some initialisation is missing, if C is created on the fly.

@dimpase
Copy link
Member

dimpase commented May 22, 2020

comment:14

This is reproducible on the ticket branch, on Fedora 30 and on Debian 10.
This works:

sage: R.<x> = QQ['x']
sage: C=HyperellipticCurve(x^5 - 2*x + 3)
sage: A,f=monsky_washnitzer.matrix_of_frobenius_hyperelliptic(C,5,3)
sage: A

but

sage: R.<x> = QQ['x']
sage: A,f = monsky_washnitzer.matrix_of_frobenius_hyperelliptic(x^5 - 2*x + 3, 5, 3)

segfaults.

@fredrik-johansson
Copy link
Contributor

comment:15

Dima, thanks for reporting the Arb doctest failures.

The slight regressions in accuracy for elliptic_pi, elliptic_pi_inc and elliptic_rj are due to using a different algorithm to compute the R_J function for certain complex parameters. The new algorithm is a bit slower and less accurate, but unlike the old algorithm always gives the right branch. These doctest results just need to be updated.

Unfortunately, the new algorithm doesn't work when phi is an inexact multiple of pi/2, leading to the NaN values. There is a workaround: acb_elliptic_pi_inc takes a pi flag to multiply the argument by pi exactly, so if you add a pi flag to the Sage wrapper and pass in phi=0.5 with pi=True in the doctest, it should give the expected result.

It's theoretically possible to fix these regressions, and I will open an Arb issue for it, but I'm not keen to spend time on it any time soon.

The other two examples look correct: converting an Arb number to an MPFR now rounds according to MPFR semantics (including rounding to infinity) instead of abort()ing when the exponents are out of range for MPFR.

@dimpase
Copy link
Member

dimpase commented May 22, 2020

comment:16

This works too:

sage: sage: R.<x> = QQ['x']
....: sage: C=HyperellipticCurve(x^5 - 2*x + 7)
....: sage: A,f = monsky_washnitzer.matrix_of_frobenius_hyperelliptic(x^5-2*x+3, 5, 3)

That is, apparently, some kind of Flint initialisation needs to happen before
the latter function can be called.

@dimpase
Copy link
Member

dimpase commented Oct 20, 2020

comment:129

rebased over 9.2.rc3

@mkoeppe
Copy link
Member

mkoeppe commented Oct 20, 2020

Work Issues: rebase on top of #30805

@mkoeppe
Copy link
Member

mkoeppe commented Oct 20, 2020

Changed dependencies from #29826, #30262 to #30805

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 21, 2020

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

14fd292prevent segfaults with flint 2.6
6898cc6update flint to 2.6.0 (alpha version), remove patches
436f15abackport flint compat patch from arb's trunk
5de748dfix and make robust related doctests
df7f4c2build/pkgs/flint: Add DESTDIR patch
ee7b2bbbuild/pkgs/arb: Bump patch level
1088accbuild/pkgs/flint: Update to 2.6.1, drop upstreamed patch
01271cbflint 2.6.3

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 21, 2020

Changed commit from 333973d to 01271cb

@dimpase
Copy link
Member

dimpase commented Oct 21, 2020

comment:132

rebased over #30805

@BrentBaccala
Copy link

comment:133

To build the current version of this branch (commit 01271cb) on Ubuntu I had to apt install libgmp-dev.

I don't think that's listed as a build requirement.

@mkoeppe
Copy link
Member

mkoeppe commented Oct 29, 2020

comment:134

Logs please

@BrentBaccala
Copy link

Attachment: flint-2.6.3.log

Build attempt on commit 01271cb

@BrentBaccala
Copy link

comment:135

I attached a log file.

There's really not much to see - a bunch of failed downloads because flint 2.6.3 isn't in any of the Sage mirrors yet, then a compile failure because it couldn't find "gmp.h", which I resolved with apt install libgmp-dev.

@dimpase
Copy link
Member

dimpase commented Oct 29, 2020

comment:136

Replying to @BrentBaccala:

I attached a log file.

There's really not much to see - a bunch of failed downloads because flint 2.6.3 isn't in any of the Sage mirrors yet,

./configure has --enable-download-from-upstream-url option, which use would allow an automatic download from the url in checksums.iniof the package.

then a compile failure because it couldn't find "gmp.h", which I resolved with apt install libgmp-dev.

look at what ./configure tries to tell at the end of its run - there should be a list of system packages to install to make one's life easier.
On Debian like 90% of Sage's (non-python) libs, as well as Python itself, etc etc, are unvendored, see #27330

@BrentBaccala
Copy link

comment:137

Replying to @dimpase:

look at what ./configure tries to tell at the end of its run - there should be a list of system packages to install to make one's life easier.
On Debian like 90% of Sage's (non-python) libs, as well as Python itself, etc etc, are unvendored, see #27330

I had to re-run configure after installing libgmp-dev, and I didn't keep the old log, but I don't remember any glaring complaints. Furthermore, I found this in the (new) config.log:

configure:37965: result: gmp-6.1.2: using system package; SPKG will not be installed

I infer from this that a SPKG is available and should have been used by the original build.

I don't know enough about Sage packaging to know how a SPKG gmp should be used by flint.

@dimpase
Copy link
Member

dimpase commented Oct 30, 2020

comment:138

Replying to @BrentBaccala:

To build the current version of this branch (commit 01271cb) on Ubuntu I had to apt install libgmp-dev.

I don't think that's listed as a build requirement.

How did you start the build?

I only can see this happening on an unconfigured tree if one does not start by
running ./configure, but rather by doing something like ./sage -p flint (which indeed does not check dependencies)

@BrentBaccala
Copy link

comment:139

Replying to @dimpase:

Replying to @BrentBaccala:

To build the current version of this branch (commit 01271cb) on Ubuntu I had to apt install libgmp-dev.

I don't think that's listed as a build requirement.

How did you start the build?

I only can see this happening on an unconfigured tree if one does not start by
running ./configure, but rather by doing something like ./sage -p flint (which indeed does not check dependencies)

After git checkout, I ran ./bootstrap, then ./configure, then MAKE="make -j8" make.

@dimpase
Copy link
Member

dimpase commented Oct 30, 2020

comment:140

could be stale leftovers - how about make bootstrap-clean && make distclean ?
(for the former possibly #30795 is needed)

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 6, 2020

Changed commit from 01271cb to 7681b1a

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 6, 2020

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

6b3a2edupdate flint to 2.6.0 (alpha version), remove patches
b53bf8efix and make robust related doctests
7731c60build/pkgs/flint: Add DESTDIR patch
24ff512build/pkgs/flint: Update to 2.6.1, drop upstreamed patch
7681b1aflint 2.6.3

@dimpase
Copy link
Member

dimpase commented Nov 6, 2020

comment:142

rebased over 9.3.beta0, squashed few things. Let us get this in please!

In particular, here are patches that need to fix doctest on distros that ship flint 2.6.3 already, e.g. Homebrew, Ubuntu 20.10.

@dimpase
Copy link
Member

dimpase commented Nov 6, 2020

Changed work issues from rebase on top of #30805 to none

@vbraun
Copy link
Member

vbraun commented Nov 15, 2020

Changed branch from public/packages/flint260 to 7681b1a

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants