#30752 closed defect (fixed)

switch the default mp library to gmp

Reported by: dimpase Owned by:
Priority: critical Milestone: sage-9.3
Component: build: configure Keywords:
Cc: jhpalmieri, mkoeppe, mjo Merged in:
Authors: Dima Pasechnik, Matthias Koeppe Reviewers: Matthias Koeppe, Dima Pasechnik
Report Upstream: N/A Work issues:
Branch: d2500f1 (Commits, GitHub, GitLab) Commit: d2500f18354e5aa37bef1ee194e6ed3335874f2e
Dependencies: Stopgaps:

Status badges

Description

currently the default mp library is mpir: the default --with-mp is system, and in build/pkgs/mpir/spkg-configure.m4 we have

SAGE_SPKG_CONFIGURE([mpir], [
dnl Implement cases for what to do on different options here
    case "$with_mp" in
        system)
            AC_CHECK_HEADER(gmp.h, [], [sage_spkg_install_mpir=yes])
...

dnl Set SAGE_MP_LIBRARY depending on the with_mp option
    case "$with_mp" in
    mpir|system)
        SAGE_MP_LIBRARY=mpir
        ;;
    gmp)
        SAGE_MP_LIBRARY=gmp

as mpir is getting more and more bitrotten, this should be switched to gmp as the default.

Also,

  --with-mp=system        use the system GMP as multiprecision library, if
                          possible (default)

is a lie, as can be seen from above, the default is mpir, not gmp.

Change History (31)

comment:1 Changed 19 months ago by dimpase

  • Authors set to Dima Pasechnik
  • Branch set to u/dimpase/packages/make_gmp_default_mp
  • Commit set to 3af67388be690e42d736efed264267c8cbf9c8c9
  • Milestone changed from sage-9.3 to sage-9.2
  • Priority changed from major to critical

marking it critical, as it corrects the ./configure -h misleading info.


New commits:

3af6738make gmp (and not mpir) the default mp lib

comment:2 Changed 19 months ago by dimpase

  • Status changed from new to needs_review

the reporting is still a bit misleading:

...
gmp-6.1.2:                                   using system package; SPKG will not be installed
...
mpir-3.0.0-644faf502c56f97d9accd301965fc57d6ec70868.p0:no suitable system package; will be installed as an SPKG
...

but this not a regression.

comment:3 Changed 19 months ago by jhpalmieri

In principle this is okay, and I'm testing it on OS X, but I think it should be merged early in a release, not late in this one, since it needs testing on a wide variety of platforms.

comment:4 Changed 19 months ago by dimpase

  • Milestone changed from sage-9.2 to sage-9.3

perhaps you'd like to try this together with #30625 - upgrade of Sage's GMP.

I've moved this to 9.3.

comment:5 Changed 19 months ago by jhpalmieri

With Xcode 12, OS X Catalina: works on its own and in combination with #30625.

comment:6 Changed 18 months ago by mkoeppe

  • Reviewers set to https://github.com/mkoeppe/sage/actions/runs/356510117

This will affect all -minimal builds - so I am testing it with GH Actions

comment:7 Changed 18 months ago by mkoeppe

  • Status changed from needs_review to needs_work

This breaks lots of -minimal builds - see for example fedora-31-minimal (https://github.com/mkoeppe/sage/runs/1381716520)

  gcc -pthread -shared -Wl,-rpath-link,/sage/local/lib -L/sage/local/lib -Wl,-rpath,/sage/local/lib -L. -Wl,-rpath-link,/sage/local/lib -L/sage/local/lib -Wl,-rpath,/sage/local/lib -Wl,-rpath-link,/sage/local/lib -L/sage/local/lib -Wl,-rpath,/sage/local/lib build/temp.linux-x86_64-3.8/cypari2/closure.o -L/sage/local/lib -L/sage/local/lib64 -L/sage/local/lib -lgmp -lpari -o build/lib.linux-x86_64-3.8/cypari2/closure.cpython-38-x86_64-linux-gnu.so -lpari
  /usr/bin/ld: cannot find -lgmp
  collect2: error: ld returned 1 exit status
  error: command 'gcc' failed with exit status 1
  Building wheel for cypari2 (setup.py): finished with status 'error'

comment:8 Changed 18 months ago by dimpase

a logic error in spkg-configure (of mpir or gmp)

2020-11-10T22:56:48.4470802Z Checking whether SageMath should install SPKG mpir...
2020-11-10T22:56:48.4471312Z checking gmp.h usability... no
2020-11-10T22:56:48.4471747Z checking gmp.h presence... no
2020-11-10T22:56:48.4472119Z checking for gmp.h... no
2020-11-10T22:56:48.4472533Z checking gmpxx.h usability... no
2020-11-10T22:56:48.4472984Z checking gmpxx.h presence... no
2020-11-10T22:56:48.4473541Z checking for gmpxx.h... no
2020-11-10T22:56:48.4473965Z checking for library containing __gmpq_cmp_z... no
2020-11-10T22:56:48.4474813Z configure: will use system package and not install SPKG mpir
2020-11-10T22:56:48.4475426Z using gmp SPKG (via --with-mp=gmp)
2020-11-10T22:56:48.4475906Z -----------------------------------------------------------------------------
2020-11-10T22:56:48.4476313Z Checking whether SageMath should install SPKG gmp...
2020-11-10T22:56:48.4476849Z configure: will use system package and not install SPKG gmp

somehow it thinks that system GMP is available.

comment:9 Changed 18 months ago by git

  • Commit changed from 3af67388be690e42d736efed264267c8cbf9c8c9 to a1b552cfa3ac2c732f1ded4c2aad9d149bba8efb

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

25de198make gmp (and not mpir) the default mp lib
a1b552ccorrect logic for spkg-configures of gmp and mpir

comment:10 Changed 18 months ago by dimpase

  • Status changed from needs_work to needs_review

fixed this (and rebased over latest beta). Tested with/without gmp present, with various options for --with-mp=, all seems to work.

comment:11 Changed 18 months ago by mkoeppe

  • Reviewers changed from https://github.com/mkoeppe/sage/actions/runs/356510117 to https://github.com/mkoeppe/sage/actions/runs/360142559

comment:12 Changed 18 months ago by dimpase

this round of GH actions looks better, several "minimal" jobs completed successfully. These that failed (e.g. ubunuty-trusty) seemed to have failed for an unrelated reason. Although it's hart to see atm, for the latter one can only see that sagelib build failed somewhere, but the full log is not there - is it supposed to become available at some point after the run completes?

comment:13 follow-up: Changed 18 months ago by mkoeppe

The full logs are now available as the run has completed

comment:14 in reply to: ↑ 13 Changed 18 months ago by dimpase

Replying to mkoeppe:

The full logs are now available as the run has completed

I don't know where to find full untruncated logs, such as install.log for e.g. docker (ubuntu-trusty, minimal). "last whatever lines" do not show the real error.

comment:15 follow-up: Changed 18 months ago by mkoeppe

comment:16 in reply to: ↑ 15 Changed 18 months ago by dimpase

Replying to mkoeppe:

In the build artifacts listed at https://github.com/mkoeppe/sage/actions/runs/360142559, such as this one: https://github.com/mkoeppe/sage/suites/1493083844/artifacts/26253188 for ubuntu-trusty-minimal

OK, thanks for pointing to the right direction. In this case the error is about a giac header, as can be seen in logs/pkgs/sagelib-9.3.beta1.log

[197/523] creating build/temp.linux-x86_64-3.8/build/cythonized/sage/libs/giac
gcc -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -Wno-unused -fPIC -Isage/libs/giac -I./sage/cpython -I/sage/local/lib/python3.8/site-packages/cysignals -I/sage/build/pkgs/sagelib/src -I/sage/l
ocal/include/python3.8 -I/sage/local/lib/python3.8/site-packages/numpy/core/include -Ibuild/cythonized -I/sage/local/include/python3.8 -c build/cythonized/sage/libs/giac/giac.cpp -o build/temp.linux-x86_64-3.8/bu
ild/cythonized/sage/libs/giac/giac.o -fno-strict-aliasing -DCYTHON_CLINE_IN_TRACEBACK=1 -fno-tree-copyrename -std=c++11
build/cythonized/sage/libs/giac/giac.cpp:676:23: fatal error: giac/giac.h: No such file or directory
 #include "giac/giac.h"
                       ^
compilation terminated.

Looks as if -I$SAGE_LOCAL/include is missing in CFLAGS, or something like this. Nothing to do with the branch on this ticket, anyway.

comment:17 Changed 18 months ago by mkoeppe

The problem with giac is probably just that missing dependency, fixed in #30858

comment:18 Changed 18 months ago by mkoeppe

  • Reviewers changed from https://github.com/mkoeppe/sage/actions/runs/360142559 to Matthias Koeppe
  • Status changed from needs_review to positive_review

comment:19 Changed 18 months ago by vbraun

  • Branch changed from u/dimpase/packages/make_gmp_default_mp to a1b552cfa3ac2c732f1ded4c2aad9d149bba8efb
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:20 Changed 18 months ago by vbraun

  • Commit a1b552cfa3ac2c732f1ded4c2aad9d149bba8efb deleted
  • Resolution fixed deleted
  • Status changed from closed to new

On OSX:

vbraun@osx Sage % ./sage -p gmp                       
Found local metadata for gmp-6.2.0
Using cached file /Users/vbraun/Sage/upstream/gmp-6.2.0.tar.xz
gmp-6.2.0
====================================================
Setting up build directory for gmp-6.2.0
Error: Unknown file type: /Users/vbraun/Sage/upstream/gmp-6.2.0.tar.xz
Finished extraction

The inability to extract xz is apparently ignored, but obviously install fails sooner or later.

comment:21 Changed 18 months ago by vbraun

  • Branch changed from a1b552cfa3ac2c732f1ded4c2aad9d149bba8efb to u/dimpase/packages/make_gmp_default_mp
  • Commit set to a1b552cfa3ac2c732f1ded4c2aad9d149bba8efb

New commits:

25de198make gmp (and not mpir) the default mp lib
a1b552ccorrect logic for spkg-configures of gmp and mpir

comment:22 Changed 18 months ago by jhpalmieri

What does this have to do with this ticket? If you can't extract xz files, then you can't extract tarballs for Python, symmetrica, gcc, and gmp.

comment:23 Changed 18 months ago by mkoeppe

These other packages declare a dependency on xz (see #30559)

comment:24 Changed 18 months ago by jhpalmieri

But on OS X, the included version of Python 3, at least as of 10.15.7, can extract .tar.xz files using its tarfile module. It looks like this feature was added in Python 3.3. We shouldn't need the xz executable for this, unless the system only has an old version of Python. Rather than using #30948, or maybe in addition, maybe we should modify the code in sage_bootstrap.uncompress.tar_file that handles xz files.

comment:25 follow-up: Changed 18 months ago by mkoeppe

That's true, but as of #29890, we prefer system python 2 because with python3 we have run into SSL problems. (The correct fix is discussed in the ticket description of #29418.)

comment:26 in reply to: ↑ 25 Changed 18 months ago by mkoeppe

Replying to mkoeppe:

The correct fix is discussed in the ticket description of #29418.

I've opened #30950 for this -- but for the present ticket, let's just add xz to the dependencies.

comment:27 Changed 18 months ago by mkoeppe

  • Branch changed from u/dimpase/packages/make_gmp_default_mp to u/mkoeppe/packages/make_gmp_default_mp

comment:28 Changed 18 months ago by mkoeppe

  • Commit changed from a1b552cfa3ac2c732f1ded4c2aad9d149bba8efb to d2500f18354e5aa37bef1ee194e6ed3335874f2e
  • Status changed from new to needs_review

New commits:

deb738cMerge tag '9.3.beta2' into t/30752/packages/make_gmp_default_mp
d2500f1build/pkgs/gmp/dependencies: Add xz

comment:29 Changed 18 months ago by mkoeppe

  • Authors changed from Dima Pasechnik to Dima Pasechnik, Matthias Koeppe
  • Reviewers changed from Matthias Koeppe to Matthias Koeppe, ...

comment:30 Changed 18 months ago by dimpase

  • Reviewers changed from Matthias Koeppe, ... to Matthias Koeppe, Dima Pasechnik
  • Status changed from needs_review to positive_review

lgtm

comment:31 Changed 18 months ago by vbraun

  • Branch changed from u/mkoeppe/packages/make_gmp_default_mp to d2500f18354e5aa37bef1ee194e6ed3335874f2e
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.