sage.rings.integer, integer_ring: Remove dependencies on sage.libs.ntl

[mkoeppe]

The two Cython modules depend on NTL in a very minor way that could be eliminated:

• sage.rings.integer.Integer depends on sage.libs.ntl to provide method _to_ZZ.

• sage.rings.integer_ring._coerce_ZZ depends on sage.libs.ntl to provide the method _coerce_ZZ.

These methods are only used in sage.libs.ntl and in sage.rings.number_field.number_field_element and rings.polynomial.polynomial_zz_pex.

In this ticket, we remove these two methods and replace all uses by their definition (= 1 or 2 lines).

CC: @videlec @fchapoton @alexjbest @mezzarobba @kliem @mwageringel @tscrim

Component: refactoring

Author: Matthias Koeppe

Branch/Commit: 62e2aa9

Reviewer: Travis Scrimshaw

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

[kiwifb]

comment:1

Hum, sage.rings.integer.integer and sage.rings.integer_ring don't link to ntl directly at least.

readelf -d /usr/lib/python3.7/site-packages/sage/rings/integer_ring.cpython-37m-x86_64-linux-gnu.so 

Dynamic section at offset 0x35ca0 contains 26 entries:
  Tag        Type                         Name/Value
 0x0000000000000001 (NEEDED)             Shared library: [libgmp.so.10]
 0x0000000000000001 (NEEDED)             Shared library: [libpython3.7m.so.1.0]
 0x0000000000000001 (NEEDED)             Shared library: [libc.so.6]
 0x000000000000000c (INIT)               0x7000

and

readelf -d /usr/lib/python3.7/site-packages/sage/rings/finite_rings/element_givaro.cpython-37m-x86_64-linux-gnu.so 

Dynamic section at offset 0x4db78 contains 30 entries:
  Tag        Type                         Name/Value
 0x0000000000000001 (NEEDED)             Shared library: [libgivaro.so.9]
 0x0000000000000001 (NEEDED)             Shared library: [libpari-gmp.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [libgmp.so.10]
 0x0000000000000001 (NEEDED)             Shared library: [libstdc++.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [libpython3.7m.so.1.0]
 0x0000000000000001 (NEEDED)             Shared library: [libgcc_s.so.1]
 0x0000000000000001 (NEEDED)             Shared library: [libc.so.6]
 0x000000000000000c (INIT)               0xd000

on Gentoo compiled with --as-needed. Some headers may still be used though.

[mkoeppe]

comment:2

It's a compile time dependency. It's possible that it is all macros or inlined functions.

[mkoeppe]

comment:5

Would it make sense to make these two methods functions in sage.libs.ntl.conversion?

[mkoeppe]

Branch: u/mkoeppe/sage_rings_integer__integer_ring__remove_dependencies_on_sage_libs_ntl

[mkoeppe]

Commit: cef2208

[mkoeppe]

New commits:

cef2208

sage.rings.integer.Integer._to_ZZ: Remove, replace uses by calls to mpz_to_ZZ

[sagetrac-git]

Changed commit from cef2208 to 15c4ccf

[sagetrac-git]

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

15c4ccf

sage.rings.integer_ring.IntegerRing_class._coerce_ZZ: Remove, replaces uses by calls to ZZ_to_mpz

[mkoeppe]

Author: Matthias Koeppe

[mkoeppe]

Dependencies: #29786

[sagetrac-git]

Changed commit from 15c4ccf to 5e9f00c

[sagetrac-git]

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

aa75810	src/module_list.py: Move options for most Extensions in sage.rings to distutils directives
43a9b16	src/sage/misc/sageinspect.py: Fix up doctest that depends on a modified source file
a5bc828	src/sage/misc/sageinspect.py: Fixup fixup
1baaa68	src/sage/misc/sageinspect.py: Remove unused import
6734969	Merge branch 't/29786/move_extension_options_from_src_module_list_py_to__distutils___directives_in_the_individual_files__part_4__sage_rings_' into t/29911/sage_rings_integer__integer_ring__remove_dependencies_on_sage_libs_ntl
5e9f00c	src/sage/rings/integer*.pyx: Remove 'libraries = ntl'

[mkoeppe]

Changed dependencies from #29786 to none

[mkoeppe]

comment:15

The dependency has been merged in 9.2.beta3. Needs review

[mwageringel]

comment:16

Replying to @mkoeppe:

Would it make sense to make these two methods functions in sage.libs.ntl.conversion?

This sounds like a good idea to me. Why do you remove the calls to sig_on/sig_off?

[sagetrac-git]

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

95cf15c	sage.rings.integer.Integer._to_ZZ: Remove, replace uses by calls to mpz_to_ZZ
0915137	sage.rings.integer_ring.IntegerRing_class._coerce_ZZ: Remove, replaces uses by calls to ZZ_to_mpz
8468ad1	src/sage/rings/integer*.pyx: Remove 'libraries = ntl'

[sagetrac-git]

Changed commit from 5e9f00c to 8468ad1

[mkoeppe]

comment:18

Rebased on 9.2.beta5

[mkoeppe]

comment:19

Replying to @mwageringel:

Why do you remove the calls to sig_on/sig_off?

Good question. I am not sure against what kind of errors these calls were supposed to be guarding.

[mkoeppe]

comment:21

Hoping for a Cython expert to take a look...

[sagetrac-git]

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

62e2aa9

Merge tag '9.2.beta6' into t/29911/sage_rings_integer__integer_ring__remove_dependencies_on_sage_libs_ntl

[sagetrac-git]

Changed commit from 8468ad1 to 62e2aa9

[tscrim]

comment:23

While I would not count myself as a Cython expert, my guess would be if the denominator was somehow not something that could be handled? Perhaps some kind of overflow or unable to allocate memory? I am really unsure.

[tscrim]

Reviewer: Travis Scrimshaw

[tscrim]

comment:24

Well, let's put this into practice as I cannot find a reason for it. Well, perhaps we should just put it into Sage to get some broader testing as it seems safe enough to me.

[mkoeppe]

comment:25

Thanks!

[vbraun]

Changed branch from u/mkoeppe/sage_rings_integer__integer_ring__remove_dependencies_on_sage_libs_ntl to 62e2aa9