Opened 3 years ago

Closed 18 months ago

#29339 closed defect (invalid)

Fix NTL spkg-configure.m4 so it rejects NTLs built with NTL_THREADS

Reported by: Matthias Köppe Owned by:
Priority: major Milestone: sage-duplicate/invalid/wontfix
Component: build: configure Keywords:
Cc: Dima Pasechnik, François Bissey, Markus Wageringel, John Palmieri, Jean-Pierre Flori Merged in:
Authors: Reviewers: Dima Pasechnik
Report Upstream: N/A Work issues:
Branch: u/mkoeppe/fix_ntl_spkg_configure_m4_so_it_rejects_ntls_built_with_ntl_threads__without_ntl_gmp_lip__without_ntl_gf2x_lib (Commits, GitHub, GitLab) Commit: c3bc092ba9e7bea69092085b65231212f0367f6d
Dependencies: Stopgaps:

Status badges

Description (last modified by Matthias Köppe)

... such as homebrew's NTL (#29104).

Builds with NTL_THREADS breaks Singular (as observed in #29104).

In fact, as of the NTL upgrade ticket #20590, we build the NTL SPKG with NTL_THREADS=off. (On that ticket, it was noted "we cannot take advantage of the threading until a number of things are resolved. Threading requires C++11 and a number of packages are behind the curve.")

In a follow-up ticket we may consider issuing a warning if NTL is configured without NTL_GMP_LIP, without NTL_GF2X_LIB, which may lead to a performance degradation.

Change History (40)

comment:1 Changed 3 years ago by Dima Pasechnik

the wise thing is to enable TLS as much as possible, IMHO. The configuration with enabled TLS on NTL, pari, and flint used to work, IIRC.

comment:2 Changed 3 years ago by Dima Pasechnik

I also don't think gf2x should be a hard requirement. It is entirely optional for NTL.

comment:3 Changed 3 years ago by Matthias Köppe

Do you mean NTL_TLS_HACK?

comment:4 Changed 3 years ago by Matthias Köppe

homebrew's NTL is configured with NTL_TLS_HACK and NTL_THREADS, and I am getting the reported error with Singular. What do you suggest?

comment:6 Changed 3 years ago by Matthias Köppe

Cc: Jean-Pierre Flori added
Description: modified (diff)

comment:7 Changed 3 years ago by Dima Pasechnik

see #27764

comment:8 Changed 3 years ago by Matthias Köppe

Branch: u/mkoeppe/fix_ntl_spkg_configure_m4_so_it_rejects_ntls_built_with_ntl_threads__without_ntl_gmp_lip__without_ntl_gf2x_lib

comment:9 Changed 3 years ago by Matthias Köppe

Authors: Matthias Koeppe
Commit: 96553db01783fcd6c86e451b0b16b4bcb88140f7
Status: newneeds_review

New commits:

96553dbbuild/pkgs/ntl/spkg-configure.m4: Check for NTL configuration

comment:10 Changed 3 years ago by Dima Pasechnik

once again - some distros build NTL without gf2x support, and it results in some deterioration of performance, but this is not something Sage should worry too much. Print a warning if you must.

After all, it is by far not the only place where the performance is suboptimal on pre-build installs, untuned for the hardware, say.

Last edited 3 years ago by Dima Pasechnik (previous) (diff)

comment:11 Changed 3 years ago by Matthias Köppe

Description: modified (diff)
Status: needs_reviewneeds_work
Summary: Fix NTL spkg-configure.m4 so it rejects NTLs built with NTL_THREADS, without NTL_GMP_LIP, without NTL_GF2X_LIBFix NTL spkg-configure.m4 so it rejects NTLs built with NTL_THREADS

comment:12 Changed 3 years ago by git

Commit: 96553db01783fcd6c86e451b0b16b4bcb88140f7c3bc092ba9e7bea69092085b65231212f0367f6d

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

c3bc092Remove tests for NTL_GMP_LIP, NTL_GF2X_LIB

comment:13 Changed 3 years ago by Matthias Köppe

Status: needs_workneeds_review

comment:14 in reply to:  10 Changed 3 years ago by Matthias Köppe

Replying to dimpase:

once again - some distros build NTL without gf2x support, and it results in some deterioration of performance, but this is not something Sage should worry too much. Print a warning if you must.

OK, I have removed the tests

comment:15 Changed 3 years ago by Dima Pasechnik

IMHO also pari/gp is typically built with threads on by distros. This means that TLS is enabled for pari/gp.

We may end up with having to build all the libs...

comment:16 Changed 3 years ago by Matthias Köppe

I don't see such problems in my tests

comment:17 Changed 3 years ago by Dima Pasechnik

do you test with options forcing the use of system packages?

comment:18 Changed 3 years ago by Matthias Köppe

Not recently, will do

comment:19 Changed 3 years ago by Matthias Köppe

However, you can look for the new messages configure: Hint: The following SPKGs did not find equivalent system packages such as (https://github.com/mkoeppe/sage/runs/512242876) for ubuntu-focal-standard:

configure: Hint: The following SPKGs did not find equivalent system packages:
configure:   arb cbc cmake eclib fflas_ffpack flint fplll isl libatomic_ops libsemigroups ninja_build ntl perl_term_readline_gnu
checking for the package system in use... debian
configure: Hint: Installing the following system packages is recommended and may avoid building some of the above SPKGs from source:
configure:   $ sudo apt-get install libflint-arb-dev coinor-cbc coinor-libcbc-dev cmake libec-dev libflint-dev libisl-dev ninja-build libntl-dev libterm-readline-gnu-perl
configure: After installation, re-run configure using:
configure:   $ ./config.status --recheck && ./config.status

comment:20 Changed 3 years ago by Dima Pasechnik

I think that the threads issue should be handled on per platform basis. If threads are not working on MacOS it does not mean that Sage on Linux should be made single-thread.

comment:21 Changed 3 years ago by Matthias Köppe

Do you know a platform that actually builds NTL with threads?

comment:22 in reply to:  21 Changed 3 years ago by François Bissey

Replying to mkoeppe:

Do you know a platform that actually builds NTL with threads?

Gentoo.

comment:23 Changed 3 years ago by Matthias Köppe

Well, does sage-the-distribution work with Gentoo's NTL?

comment:24 Changed 3 years ago by Dima Pasechnik

yes it does, I think.

comment:25 Changed 3 years ago by Matthias Köppe

Shouldn't there be a ticket that enables NTL threads for Sage-the-distribution then? The NTL upgrade ticket #20590 disabled threads explicitly. So I assume there was a difficulty -- and there is no evidence that it has been resolved.

comment:26 Changed 3 years ago by Dima Pasechnik

4 years ago the Python's single-thread religion still had many followers :-)

comment:27 Changed 3 years ago by Matthias Köppe

No, seriously, when our own installation scripts avoid NTL's threads, then I think it is quite appropriate that the spkg-configure checks for that.

comment:28 Changed 3 years ago by Dima Pasechnik

by the way, Fedora 30 (and on newer Fedoras too) also provides multithreaded NTL, and Sage happily uses it, tests pass.

$ uname -a
Linux clpc171.cs.ox.ac.uk 5.2.18-200.fc30.x86_64 #1 SMP Tue Oct 1 13:14:07 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
$ ldd /lib64/libntl.so
        linux-vdso.so.1 (0x00007ffe9affd000)
        libgmp.so.10 => /lib64/libgmp.so.10 (0x00007fabe7899000)
        libgf2x.so.1 => /lib64/libgf2x.so.1 (0x00007fabe7888000)
        libpthread.so.0 => /lib64/libpthread.so.0 (0x00007fabe7867000)
        libstdc++.so.6 => /lib64/libstdc++.so.6 (0x00007fabe766d000)
        libm.so.6 => /lib64/libm.so.6 (0x00007fabe7527000)
        libc.so.6 => /lib64/libc.so.6 (0x00007fabe7361000)
        /lib64/ld-linux-x86-64.so.2 (0x00007fabe7c33000)
        libgcc_s.so.1 => /lib64/libgcc_s.so.1 (0x00007fabe7345000)
Last edited 3 years ago by Dima Pasechnik (previous) (diff)

comment:29 in reply to:  27 ; Changed 3 years ago by Dima Pasechnik

Replying to mkoeppe:

No, seriously, when our own installation scripts avoid NTL's threads, then I think it is quite appropriate that the spkg-configure checks for that.

I don't see why. Anyhow, this problem (inability to use multithreaded NTL) seems to be MacOS-only, and probably related to #27764

comment:30 Changed 3 years ago by François Bissey

We historically had an issue before defaulting to C++11 but we should be able to move ahead in my opinion. I am expecting distros to all be threaded at this stage. It is just a matter of testing the insane number of corner cases.

comment:31 in reply to:  30 Changed 3 years ago by Dima Pasechnik

Replying to fbissey:

We historically had an issue before defaulting to C++11 but we should be able to move ahead in my opinion. I am expecting distros to all be threaded at this stage. It is just a matter of testing the insane number of corner cases.

it seems to be that Debian is not providing multithreaded NTL (and the related libs, such as Flint, are single-threaded too).

comment:32 Changed 3 years ago by Markus Wageringel

For what it is worth, on macOS 10.13.6, Homebrew's NTL works for me without any problems. I last checked with a clean build of 9.1.beta4 and ptestlong passed.

comment:33 in reply to:  30 Changed 3 years ago by Matthias Köppe

Replying to fbissey:

We historically had an issue before defaulting to C++11 but we should be able to move ahead in my opinion. I am expecting distros to all be threaded at this stage. It is just a matter of testing the insane number of corner cases.

I have created ticket #29340 (Enable threads in NTL) for that, in case someone would like to work on it.

comment:34 in reply to:  29 Changed 3 years ago by Matthias Köppe

Replying to dimpase:

Replying to mkoeppe:

No, seriously, when our own installation scripts avoid NTL's threads, then I think it is quite appropriate that the spkg-configure checks for that.

I don't see why. Anyhow, this problem (inability to use multithreaded NTL) seems to be MacOS-only, and probably related to #27764

#27764 does not fix the problem on macOS, I've already tested that

comment:35 Changed 3 years ago by Matthias Köppe

Status: needs_reviewneeds_info

See #29366 (arch)

comment:36 Changed 3 years ago by Matthias Köppe

Milestone: sage-9.1sage-9.2

comment:37 Changed 2 years ago by Matthias Köppe

Milestone: sage-9.2sage-9.3

comment:38 Changed 22 months ago by Matthias Köppe

Authors: Matthias Koeppe
Milestone: sage-9.3sage-duplicate/invalid/wontfix
Status: needs_infoneeds_review

I think this is no longer needed

comment:39 Changed 22 months ago by Dima Pasechnik

Reviewers: Dima Pasechnik
Status: needs_reviewpositive_review

ok

comment:40 Changed 18 months ago by Matthias Köppe

Resolution: invalid
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.