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

FPLLL 5.4.0 and FPyLLL 0.5.4 #31025

Closed
malb opened this issue Dec 7, 2020 · 49 comments
Closed

FPLLL 5.4.0 and FPyLLL 0.5.4 #31025

malb opened this issue Dec 7, 2020 · 49 comments

Comments

@malb
Copy link
Member

malb commented Dec 7, 2020

Tarball URLs - see checksums.ini

CC: @kiwifb

Component: packages: standard

Keywords: sd111

Author: Martin Albrecht

Branch: 29fcb34

Reviewer: Matthias Koeppe

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

@malb malb added this to the sage-9.3 milestone Dec 7, 2020
@malb
Copy link
Member Author

malb commented Dec 7, 2020

@malb
Copy link
Member Author

malb commented Dec 7, 2020

Commit: 8faa924

@malb
Copy link
Member Author

malb commented Dec 7, 2020

Author: Martin Albrecht

@malb
Copy link
Member Author

malb commented Dec 7, 2020

New commits:

8faa924FPLLL 5.4.0 and FPyLLL 0.5.3.dev0

@malb

This comment has been minimized.

@mkoeppe
Copy link
Member

mkoeppe commented Dec 8, 2020

comment:4

build/pkgs/fpylll/checksums.ini needs upstream_url - see other python packages for the preferred URL format

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 8, 2020

Changed commit from 8faa924 to 0ab1d0e

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 8, 2020

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

0ab1d0eadd upstream_url

@malb
Copy link
Member Author

malb commented Dec 8, 2020

comment:6

Ta. Fixed.

@mkoeppe
Copy link
Member

mkoeppe commented Dec 10, 2020

Changed keywords from none to sd111

@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Member

mkoeppe commented Dec 10, 2020

@mkoeppe
Copy link
Member

mkoeppe commented Dec 10, 2020

@mkoeppe
Copy link
Member

mkoeppe commented Dec 11, 2020

comment:11

On local-conda-forge-ubuntu-standard (https://github.com/mkoeppe/sage/runs/1533944840)

  build/src/fpylll/fplll/enumeration.cpp: In function 'PyObject* __pyx_pf_6fpylll_5fplll_11enumeration_11Enumeration_6get_nodes(__pyx_obj_6fpylll_5fplll_11enumeration_Enumeration*, PyObject*)':
  build/src/fpylll/fplll/enumeration.cpp:7430:99: error: no matching function for call to 'fplll::Enumeration<fplll::Z_NR<__mpz_struct [1]>, fplll::FP_NR<double> >::get_nodes(int&)'
       __pyx_t_3 = __Pyx_PyInt_From_unsigned_long(__pyx_v_self->_core.mpz_d->get_nodes(__pyx_v__level)); if (unlikely(!__pyx_t_3)) __PYX_ERR(0, 576, __pyx_L1_error)
                                                                                                     

@kiwifb
Copy link
Member

kiwifb commented Dec 11, 2020

comment:12

Works fine in Gentoo. I must I never know where to find the appropriate logs in your runs. Sometimes it is easy and sometimes I just get lost. I found the good stuff in the raw logs.

2020-12-11T03:40:06.7154190Z   building 'fpylll.fplll.enumeration' extension
2020-12-11T03:40:06.7164990Z   /home/runner/work/sage/sage/.tox/local-conda-forge-ubuntu-standard/conda/bin/x86_64-conda-linux-gnu-cc -Wno-unused-result -Wsign-compare -DNDEBUG -fwrapv -O2 -Wall -Wstrict-prototypes -march=nocona -mtune=haswell -ftree-vectorize -fPIC -fstack-protector-strong -fno-plt -O2 -pipe -march=nocona -mtune=haswell -ftree-vectorize -fPIC -fstack-protector-strong -fno-plt -O2 -pipe -march=nocona -mtune=haswell -ftree-vectorize -fPIC -fstack-protector-strong -fno-plt -O2 -ffunction-sections -pipe -isystem /home/runner/work/sage/sage/.tox/local-conda-forge-ubuntu-standard/conda/include -DNDEBUG -D_FORTIFY_SOURCE=2 -O2 -isystem /home/runner/work/sage/sage/.tox/local-conda-forge-ubuntu-standard/conda/include -fPIC -Isrc/fpylll/fplll -I/home/runner/work/sage/sage/.tox/local-conda-forge-ubuntu-standard/local/lib/python3.8/site-packages/cysignals -I/home/runner/work/sage/sage/.tox/local-conda-forge-ubuntu-standard/local/include -I/home/runner/work/sage/sage/.tox/local-conda-forge-ubuntu-standard/local/lib/python3.8/site-packages/numpy/core/include -I/home/runner/work/sage/sage/.tox/local-conda-forge-ubuntu-standard/local/include -I/home/runner/work/sage/sage/.tox/local-conda-forge-ubuntu-standard/conda/include/python3.8 -c build/src/fpylll/fplll/enumeration.cpp -o build/temp.linux-x86_64-3.8/build/src/fpylll/fplll/enumeration.o -std=c++11 -fvisibility-inlines-hidden -std=c++17 -fmessage-length=0 -march=nocona -mtune=haswell -ftree-vectorize -fPIC -fstack-protector-strong -fno-plt -O2 -ffunction-sections -pipe -isystem /home/runner/work/sage/sage/.tox/local-conda-forge-ubuntu-standard/conda/include
2020-12-11T03:40:06.7174509Z   cc1plus: warning: command line option '-Wstrict-prototypes' is valid for C/ObjC but not for C++
2020-12-11T03:40:06.7176260Z   build/src/fpylll/fplll/enumeration.cpp: In function 'PyObject* __pyx_pf_6fpylll_5fplll_11enumeration_11Enumeration_6get_nodes(__pyx_obj_6fpylll_5fplll_11enumeration_Enumeration*, PyObject*)':
2020-12-11T03:40:06.7178575Z   build/src/fpylll/fplll/enumeration.cpp:7430:99: error: no matching function for call to 'fplll::Enumeration<fplll::Z_NR<__mpz_struct [1]>, fplll::FP_NR<double> >::get_nodes(int&)'
2020-12-11T03:40:06.7180107Z        __pyx_t_3 = __Pyx_PyInt_From_unsigned_long(__pyx_v_self->_core.mpz_d->get_nodes(__pyx_v__level)); if (unlikely(!__pyx_t_3)) __PYX_ERR(0, 576, __pyx_L1_error)

I have a feeling -std=c++17 is responsible for this, I compile with c++11 in Gentoo (probably a default somewhere).

@malb
Copy link
Member Author

malb commented Dec 11, 2020

comment:13

Is there perhaps a race condition that FPyLLL is built before the new FPLLL?

@kiwifb
Copy link
Member

kiwifb commented Dec 11, 2020

comment:14

Replying to @malb:

Is there perhaps a race condition that FPyLLL is built before the new FPLLL?

Looking at the log again, I'd say it looks like it!

@mkoeppe
Copy link
Member

mkoeppe commented Dec 11, 2020

comment:15

This build is accepting a system package

Checking whether SageMath should install SPKG fplll...
checking whether any of mpfr is installed as or will be installed as SPKG... no
checking for fplll >= 5.3... yes
configure: will use system package and not install SPKG fplll

If the new fpylll has other version requirements, build/pkgs/fplll/spkg-configure.m4 needs to be changed

@mkoeppe
Copy link
Member

mkoeppe commented Dec 11, 2020

comment:16

Same failure also on fedora-32-standard https://github.com/mkoeppe/sage/runs/1533944016 and other builds

@mkoeppe
Copy link
Member

mkoeppe commented Dec 11, 2020

comment:17

If practical at all, I would suggest to upstream to allow the Python bindings to accept a range of fplll versions...

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 11, 2020

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

9eb14e4update required FPLLL version

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 11, 2020

Changed commit from 0ab1d0e to 9eb14e4

@malb
Copy link
Member Author

malb commented Dec 11, 2020

comment:19

Updated accordingly, I don't think we can commit to supporting a range of FPLLL versions in FPyLLL

@mkoeppe
Copy link
Member

mkoeppe commented Dec 11, 2020

comment:20

Perhaps then an upper bound for the version needs to be set as well?

@mkoeppe
Copy link
Member

mkoeppe commented Dec 14, 2020

comment:21

Likewise, should we set a range of acceptable fpylll versions (in the install-requires of sagelib)?

@mkoeppe
Copy link
Member

mkoeppe commented Dec 15, 2020

comment:30

Based on your explanations, I think fpylll's install-requires.txt should also fix the version.

@mkoeppe
Copy link
Member

mkoeppe commented Dec 16, 2020

@malb
Copy link
Member Author

malb commented Dec 17, 2020

comment:32

This doesn't look like an FPLLL issue: https://github.com/mkoeppe/sage/runs/1566871595?check_suite_focus=true#step:7:65

@mkoeppe
Copy link
Member

mkoeppe commented Dec 17, 2020

comment:33

No, that's not related to this ticket

@mkoeppe
Copy link
Member

mkoeppe commented Dec 17, 2020

comment:34

Replying to @mkoeppe:

Based on your explanations, I think fpylll's install-requires.txt should also fix the version.

As in:

diff --git a/build/pkgs/fpylll/install-requires.txt b/build/pkgs/fpylll/install-requires.txt
index 6e7d17221c..df75cd9fa7 100644
--- a/build/pkgs/fpylll/install-requires.txt
+++ b/build/pkgs/fpylll/install-requires.txt
@@ -1 +1 @@
-fpylll >=0.5.4
+fpylll ==0.5.4

@mkoeppe
Copy link
Member

mkoeppe commented Dec 17, 2020

comment:35

I would also suggest to add some comments:

diff --git a/build/pkgs/fplll/spkg-configure.m4 b/build/pkgs/fplll/spkg-configure.m4
index 6da0045fe0..b98a71938e 100644
--- a/build/pkgs/fplll/spkg-configure.m4
+++ b/build/pkgs/fplll/spkg-configure.m4
@@ -4,6 +4,9 @@ SAGE_SPKG_CONFIGURE([fplll], [
     dnl if there's a usable system copy of fplll. Unless there's
     dnl a system that ships fplll without fplll.pc file, falling
     dnl back to a manual header/library search is pointless.
+    dnl
+    dnl Trac #31025: FPLLL/FPyLLL make no guarantee regarding compatibility
+    dnl other than "whatever versions were released at the same time should work together"
     PKG_CHECK_MODULES([FPLLL],
                       [fplll == 5.4],
                       [sage_spkg_install_fplll=no],
diff --git a/build/pkgs/fpylll/install-requires.txt b/build/pkgs/fpylll/install-requires.txt
index 6e7d17221c..4b425c186e 100644
--- a/build/pkgs/fpylll/install-requires.txt
+++ b/build/pkgs/fpylll/install-requires.txt
@@ -1 +1,3 @@
-fpylll >=0.5.4
+# Trac #31025: FPLLL/FPyLLL make no guarantee regarding compatibility
+# other than "whatever versions were released at the same time should work together"
+fpylll ==0.5.4

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 22, 2020

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

29fcb34implement suggested changes by Matthias

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 22, 2020

Changed commit from 26032b1 to 29fcb34

@mkoeppe
Copy link
Member

mkoeppe commented Dec 22, 2020

Changed reviewer from https://github.com/mkoeppe/sage/actions/runs/426748847 to Matthias Koeppe

@mkoeppe
Copy link
Member

mkoeppe commented Dec 22, 2020

comment:39

Marking it as critical because some rolling distributions are already picking up 5.4.0

@vbraun
Copy link
Member

vbraun commented Dec 27, 2020

Changed branch from u/malb/fplll_5_4_0_and_fpylll_0_5_3_dev0 to 29fcb34

@dcoudert
Copy link
Contributor

comment:41

For some reason, even after make distclean, configure is unable to find homebrew install of fplll 5.4.0 (on macOS). Then, the compilation fails on fpylll (may be a mix of fplll installations).

After modifying spkg-configure.m4 to change fplll == 5.4.0 to fplll >= 5.4.0 and adding file distros/homebrew.txt with fplll, configure finds the local install and everything goes well.

Should we open a ticket for that ?

@dcoudert
Copy link
Contributor

Changed commit from 29fcb34 to none

@mkoeppe
Copy link
Member

mkoeppe commented Dec 28, 2020

comment:42

Replying to @dcoudert:

Should we open a ticket for that ?

Yes please. What version of fplll does pkg-config report?

@dcoudert
Copy link
Contributor

comment:43

not sure how to get that, but inside config.log, I see:

## ------------------------------------------------------ ##
## Checking whether SageMath should install SPKG fplll... ##
## ------------------------------------------------------ ##
configure:19538: checking whether any of mpfr is installed as or will be installed as SPKG
configure:19547: result: no
configure:19552: checking for fplll >= 5.4
configure:19559: $PKG_CONFIG --exists --print-errors "fplll >= 5.4"
configure:19562: $? = 0
configure:19576: $PKG_CONFIG --exists --print-errors "fplll >= 5.4"
configure:19579: $? = 0
configure:19617: result: yes
configure:19629: will use system package and not install SPKG fplll

and I have fplll 5.4.0 installed by homebrew.

Follow-up on #31127

@mkoeppe
Copy link
Member

mkoeppe commented Jan 1, 2021

comment:44

Another follow up in #31146: cygwin-standard: fpylll build fails

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

5 participants