Opened 7 years ago

Closed 7 years ago

#18367 closed enhancement (fixed)

Move ntl_wrap to Sage library

Reported by: jdemeyer Owned by:
Priority: major Milestone: sage-6.8
Component: c_lib Keywords:
Cc: Merged in:
Authors: Jeroen Demeyer Reviewers: François Bissey
Report Upstream: N/A Work issues:
Branch: 0bd4c02 (Commits, GitHub, GitLab) Commit: 0bd4c02e112fb242060fd112e7f36a17c36a80e8
Dependencies: #18519 Stopgaps:

Status badges

Description


Change History (22)

comment:1 Changed 7 years ago by jdemeyer

  • Branch set to u/jdemeyer/move_ntl_wrap_to_sage_library

comment:2 Changed 7 years ago by jdemeyer

  • Commit set to bf7497a9f33239e8cd1da2037f751aaf643d8da9
  • Dependencies set to #18519

New commits:

bf7497aMove ntl_wrap to the Sage library

comment:3 Changed 7 years ago by git

  • Commit changed from bf7497a9f33239e8cd1da2037f751aaf643d8da9 to b5a143f743ee88f7e8e5a894943f25fefe2116c7

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

2ed346bRemove unneeded inclusions of cdefs.pxi
57e9278More removals of cdefs.pxi
370106dEven more removals of cdefs
1accad6Remove cdefs.pxi from .pxd/.pxi files
ec497b4Merge tag '6.8.beta0' into HEAD
b5a143fMove ntl_wrap to Sage library

comment:4 Changed 7 years ago by jdemeyer

  • Status changed from new to needs_review

comment:5 Changed 7 years ago by git

  • Commit changed from b5a143f743ee88f7e8e5a894943f25fefe2116c7 to 6cf2f45d0fef31a9d954cc04691edd9c419839bb

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

6cf2f45Merge tag '6.8.beta1' into t/18367/move_ntl_wrap_to_sage_library

comment:6 Changed 7 years ago by git

  • Commit changed from 6cf2f45d0fef31a9d954cc04691edd9c419839bb to 97215ae7497e2b510cc8df715b2340561b057af8

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

97215aeRename ntl_wrap -> ntlwrap

comment:7 Changed 7 years ago by fbissey

  • Reviewers set to François Bissey
  • Status changed from needs_review to positive_review

Looks ready to be sent to the bots to me.

comment:8 Changed 7 years ago by fbissey

  • Milestone changed from sage-6.7 to sage-6.8

comment:9 Changed 7 years ago by vbraun

  • Status changed from positive_review to needs_work

On arando, snapperkob, and arm I get this:

Deleting empty directory /home/buildslave-sage/slave/sage_git/build/src/doc/hu/a_tour_of_sage/static
Traceback (most recent call last):
  File "/home/buildslave-sage/slave/sage_git/build/src/doc/common/builder.py", line 1619, in <module>
    import sage.all
  File "/home/buildslave-sage/slave/sage_git/build/local/lib/python2.7/site-packages/sage/all.py", line 99, in <module>
    from sage.rings.all      import *
  File "/home/buildslave-sage/slave/sage_git/build/local/lib/python2.7/site-packages/sage/rings/all.py", line 68, in <module>
    from number_field.all import *
  File "/home/buildslave-sage/slave/sage_git/build/local/lib/python2.7/site-packages/sage/rings/number_field/all.py", line 7, in <module>
    from totallyreal import enumerate_totallyreal_fields_prim
  File "sage/rings/number_field/totallyreal_data.pxd", line 12, in init sage.rings.number_field.totallyreal (build/cythonized/sage/rings/number_field/totallyreal.c:10354)
  File "sage/rings/number_field/totallyreal_data.pyx", line 40, in init sage.rings.number_field.totallyreal_data (build/cythonized/sage/rings/number_field/totallyreal_data.c:11148)
  File "/home/buildslave-sage/slave/sage_git/build/local/lib/python2.7/site-packages/sage/rings/polynomial/polynomial_ring_constructor.py", line 465, in PolynomialRing
    R = _single_variate(base_ring, name, sparse, implementation)
  File "/home/buildslave-sage/slave/sage_git/build/local/lib/python2.7/site-packages/sage/rings/polynomial/polynomial_ring_constructor.py", line 539, in _single_variate
    R = m.PolynomialRing_integral_domain(base_ring, name, sparse, implementation)
  File "/home/buildslave-sage/slave/sage_git/build/local/lib/python2.7/site-packages/sage/rings/polynomial/polynomial_ring.py", line 1544, in __init__
    sparse=sparse, element_class=element_class)
  File "/home/buildslave-sage/slave/sage_git/build/local/lib/python2.7/site-packages/sage/rings/polynomial/polynomial_ring.py", line 1451, in __init__
    sparse=sparse, element_class=element_class, category=category)
  File "/home/buildslave-sage/slave/sage_git/build/local/lib/python2.7/site-packages/sage/rings/polynomial/polynomial_ring.py", line 305, in __init__
    from sage.matrix.matrix_space import is_MatrixSpace
  File "/home/buildslave-sage/slave/sage_git/build/local/lib/python2.7/site-packages/sage/matrix/matrix_space.py", line 46, in <module>
    import matrix_modn_sparse
  File "sage/matrix/matrix_integer_dense.pxd", line 11, in init sage.matrix.matrix_modn_sparse (build/cythonized/sage/matrix/matrix_modn_sparse.c:18762)
  File "sage/matrix/matrix_modn_dense_template_header.pxi", line 9, in init sage.matrix.matrix_integer_dense (build/cythonized/sage/matrix/matrix_integer_dense.c:50329)
ImportError: /home/buildslave-sage/slave/sage_git/build/local/lib/python2.7/site-packages/sage/matrix/matrix_modn_dense_float.so: undefined symbol: _ZN3NTL8ZZ_pInfoE

comment:10 Changed 7 years ago by fbissey

What gcc versions are those? It probably needs a compiler flag, the symbol comes from ZZ_p.h.

comment:11 Changed 7 years ago by vbraun

As one data point, arando is gcc version 4.8.2 (Ubuntu 4.8.2-19ubuntu1)

comment:12 Changed 7 years ago by fbissey

Hum, the symbol should be in libntl which suggests that it didn't link appropriately to it.

comment:13 Changed 7 years ago by jdemeyer

Volker, did you also include #18450 in that test run since that also affects libraries?

comment:14 Changed 7 years ago by vbraun

No, all tickets that were included are closed by now

comment:15 Changed 7 years ago by git

  • Commit changed from 97215ae7497e2b510cc8df715b2340561b057af8 to 0bd4c02e112fb242060fd112e7f36a17c36a80e8

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

0bd4c02linbox needs ntl

comment:16 Changed 7 years ago by jdemeyer

  • Status changed from needs_work to needs_review

comment:17 Changed 7 years ago by fbissey

Actually info from sage-on-gentoo which is all compiled with -as-needed suggest a slightly different approach:

fbissey@QCD-nzi3 ~ $ ldd -r /usr/lib64/python2.7/site-packages/sage/matrix/matrix_modn_dense_float.so
	linux-vdso.so.1 (0x00007fff24fff000)
	libgivaro.so.0 => /usr/lib64/libgivaro.so.0 (0x00007f318a8ce000)
	libgmp.so.10 => /usr/lib64/libgmp.so.10 (0x00007f318a65d000)
	libopenblas_openmp.so.0 => /usr/lib64/libopenblas_openmp.so.0 (0x00007f318a0d5000)
	libstdc++.so.6 => /usr/lib/gcc/x86_64-pc-linux-gnu/4.9.2/libstdc++.so.6 (0x00007f3189dc6000)
	libpython2.7.so.1.0 => /usr/lib64/libpython2.7.so.1.0 (0x00007f3189a08000)
	libm.so.6 => /lib64/libm.so.6 (0x00007f3189703000)
	libgcc_s.so.1 => /usr/lib/gcc/x86_64-pc-linux-gnu/4.9.2/libgcc_s.so.1 (0x00007f31894ec000)
	libc.so.6 => /lib64/libc.so.6 (0x00007f318914f000)
	libgmpxx.so.4 => /usr/lib64/libgmpxx.so.4 (0x00007f3188f48000)
	libpthread.so.0 => /lib64/libpthread.so.0 (0x00007f3188d2d000)
	libgomp.so.1 => /usr/lib/gcc/x86_64-pc-linux-gnu/4.9.2/libgomp.so.1 (0x00007f3188b16000)
	/lib64/ld-linux-x86-64.so.2 (0x00007f318ade4000)
	libdl.so.2 => /lib64/libdl.so.2 (0x00007f3188911000)
	libutil.so.1 => /lib64/libutil.so.1 (0x00007f318870e000)
fbissey@QCD-nzi3 ~ $ ldd -r /usr/lib64/python2.7/site-packages/sage/matrix/matrix_modn_dense_double.so
	linux-vdso.so.1 (0x00007fff91dff000)
	libgivaro.so.0 => /usr/lib64/libgivaro.so.0 (0x00007f9413287000)
	libgmp.so.10 => /usr/lib64/libgmp.so.10 (0x00007f9413016000)
	libopenblas_openmp.so.0 => /usr/lib64/libopenblas_openmp.so.0 (0x00007f9412a8e000)
	libstdc++.so.6 => /usr/lib/gcc/x86_64-pc-linux-gnu/4.9.2/libstdc++.so.6 (0x00007f941277f000)
	libpython2.7.so.1.0 => /usr/lib64/libpython2.7.so.1.0 (0x00007f94123c1000)
	libm.so.6 => /lib64/libm.so.6 (0x00007f94120bc000)
	libgcc_s.so.1 => /usr/lib/gcc/x86_64-pc-linux-gnu/4.9.2/libgcc_s.so.1 (0x00007f9411ea5000)
	libc.so.6 => /lib64/libc.so.6 (0x00007f9411b08000)
	libgmpxx.so.4 => /usr/lib64/libgmpxx.so.4 (0x00007f9411901000)
	libpthread.so.0 => /lib64/libpthread.so.0 (0x00007f94116e6000)
	libgomp.so.1 => /usr/lib/gcc/x86_64-pc-linux-gnu/4.9.2/libgomp.so.1 (0x00007f94114cf000)
	/lib64/ld-linux-x86-64.so.2 (0x00007f941379b000)
	libdl.so.2 => /lib64/libdl.so.2 (0x00007f94112ca000)
	libutil.so.1 => /lib64/libutil.so.1 (0x00007f94110c7000)

Neither of those get libntl and they don't get liblinbox either. All the libraries are resolved so they are definitely not needed. Furthermore liblinbox doesn't need libntl. Only liblinboxsage does and curiously I don't think we ever link to it.

fbissey@QCD-nzi3 ~ $ ldd -r /usr/lib64/liblinbox.so
	linux-vdso.so.1 (0x00007fff2d334000)
	libgivaro.so.0 => /usr/lib64/libgivaro.so.0 (0x00007fbb7557f000)
	libstdc++.so.6 => /usr/lib/gcc/x86_64-pc-linux-gnu/4.9.2/libstdc++.so.6 (0x00007fbb7526f000)
	libc.so.6 => /lib64/libc.so.6 (0x00007fbb74ed2000)
	libgcc_s.so.1 => /usr/lib/gcc/x86_64-pc-linux-gnu/4.9.2/libgcc_s.so.1 (0x00007fbb74cbb000)
	libgmpxx.so.4 => /usr/lib64/libgmpxx.so.4 (0x00007fbb74ab5000)
	libgmp.so.10 => /usr/lib64/libgmp.so.10 (0x00007fbb74843000)
	libm.so.6 => /lib64/libm.so.6 (0x00007fbb7453f000)
	/lib64/ld-linux-x86-64.so.2 (0x00007fbb75a18000)
fbissey@QCD-nzi3 ~ $ ldd -r /usr/lib64/liblinboxsage.so
	linux-vdso.so.1 (0x00007ffffbbcf000)
	libgmpxx.so.4 => /usr/lib64/libgmpxx.so.4 (0x00007fed8461a000)
	libntl.so.5 => /usr/lib64/libntl.so.5 (0x00007fed84260000)
	libgmp.so.10 => /usr/lib64/libgmp.so.10 (0x00007fed83fee000)
	libgf2x.so.1 => /usr/lib64/libgf2x.so.1 (0x00007fed83dd9000)
	libopenblas_openmp.so.0 => /usr/lib64/libopenblas_openmp.so.0 (0x00007fed83852000)
	liblinbox.so.0 => /usr/lib64/liblinbox.so.0 (0x00007fed8364a000)
	libgivaro.so.0 => /usr/lib64/libgivaro.so.0 (0x00007fed833ee000)
	libiml.so.0 => /usr/lib64/libiml.so.0 (0x00007fed831cd000)
	libstdc++.so.6 => /usr/lib/gcc/x86_64-pc-linux-gnu/4.9.2/libstdc++.so.6 (0x00007fed82ebd000)
	libm.so.6 => /lib64/libm.so.6 (0x00007fed82bb9000)
	libc.so.6 => /lib64/libc.so.6 (0x00007fed8281c000)
	libgcc_s.so.1 => /usr/lib/gcc/x86_64-pc-linux-gnu/4.9.2/libgcc_s.so.1 (0x00007fed82604000)
	libpthread.so.0 => /lib64/libpthread.so.0 (0x00007fed823e9000)
	libgomp.so.1 => /usr/lib/gcc/x86_64-pc-linux-gnu/4.9.2/libgomp.so.1 (0x00007fed821d2000)
	/lib64/ld-linux-x86-64.so.2 (0x00007fed84ca3000)

In the first instance Jeroen's latest commit would solve the present problem though.

comment:18 Changed 7 years ago by fbissey

  • Status changed from needs_review to positive_review

However, I will go with caution. Because #18450 could have brought something in, that I cannot see yet, in this ticket. The commit looks good to me.

comment:19 Changed 7 years ago by jdemeyer

OK, maybe the commit message was wrong, but at least the fix works...

comment:20 follow-up: Changed 7 years ago by fbissey

Interestingly you have that stuff in the commit for #17854, which now may need some rebasing?

comment:21 in reply to: ↑ 20 Changed 7 years ago by jdemeyer

Replying to fbissey:

Interestingly you have that stuff in the commit for #17854, which now may need some rebasing?

Cool git tip: if you make an identical change in two different commits, git recognizes that and it doesn't cause a conflict.

comment:22 Changed 7 years ago by vbraun

  • Branch changed from u/jdemeyer/move_ntl_wrap_to_sage_library to 0bd4c02e112fb242060fd112e7f36a17c36a80e8
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.