Opened 6 years ago
Closed 6 years ago
#21676 closed enhancement (fixed)
Upate NTL to 10.1.0
Reported by:  jpflori  Owned by:  

Priority:  major  Milestone:  sage7.5 
Component:  packages: standard  Keywords:  
Cc:  leif, fbissey, jdemeyer, vbraun  Merged in:  
Authors:  JeanPierre Flori  Reviewers:  François Bissey 
Report Upstream:  N/A  Work issues:  
Branch:  091862e (Commits, GitHub, GitLab)  Commit:  091862e2f891dc4f5144ad6fedb66ad6bfafe2fd 
Dependencies:  #21631  Stopgaps: 
Description (last modified by )
Use upstream tarball at:
Change History (32)
comment:1 Changed 6 years ago by
Description:  modified (diff) 

Status:  new → needs_review 
comment:2 Changed 6 years ago by
comment:3 Changed 6 years ago by
I would say use the new version (at the risk of volunteering to review), since that is what we are going to have to eventually anyways.
comment:4 followup: 5 Changed 6 years ago by
Definitely new version. That's what I am targeting for Gentoo. I have one last issue with testing that I need to sort out but it shouldn't have an effect on sage since sage does test "backwards" (testing after install rather than before).
comment:5 Changed 6 years ago by
Replying to fbissey:
Definitely new version. That's what I am targeting for Gentoo. I have one last issue with testing that I need to sort out but it shouldn't have an effect on sage since sage does test "backwards" (testing after install rather than before).
Victor is good, not obvious to find but he does run tests with the appropriate LD_LIBRARY_PATH
on linux at least.
comment:6 Changed 6 years ago by
Any of you have seen that while compiling singular 4?
NTLconvert.cc: In function ‘CanonicalForm convertZZ2CF(const NTL::ZZ&)’: NTLconvert.cc:509:38: error: invalid static_cast from type ‘const raw_ptr {aka _ntl_gbigint_body* const}’ to type ‘long int*’ static_cast<long *>( a.rep.rep ); // what about NTL7?
It happens with and without the "nothrow" patch to ntl
. Did I miss a patch to singular?
comment:7 Changed 6 years ago by
It vaguely reminds me of issues btw NTL and Singular 3 with template instantiation when we updated NTL to the first version using templates (rather than macros).
comment:8 Changed 6 years ago by
That was at #16882 from:
 https://trac.sagemath.org/ticket/16882#comment:29
 http://git.sagemath.org/sage.git/commit/?id=071b2546b17df057f4ac157b096b9ee0fc33e2e5
Though you can see at https://trac.sagemath.org/ticket/16882#comment:97 and following comment that at that point I had no trouble building Singular 4.
comment:9 Changed 6 years ago by
We had a chat with Victor on sagedevel I see you have seen it by now. So Victor changed something in ntl that triggers this particular error (and presumably it would also be present in singular 3). Because it is an undocumented/internal interface singular gets the failure they deserve.
comment:10 Changed 6 years ago by
Status:  needs_review → needs_work 

comment:11 Changed 6 years ago by
I did some experiments, starting with upgrading singular
to 4.0.3p4 (without touching ntl
) on top of 7.5.beta0, since it contains the fix for ntl
10+. The plan was to change one thing at a time. You would think that it being just a patch level, it would be straightforward. Well so much for that
sage t long /usr/lib64/python2.7/sitepackages/sage/modular/modform_hecketriangle/graded_ring_element.py # 52 doctests failed sage t long /usr/lib64/python2.7/sitepackages/sage/rings/asymptotic/asymptotics_multivariate_generating_functions.py # 29 doctests failed sage t long /usr/lib64/python2.7/sitepackages/sage/modular/modform_hecketriangle/readme.py # 10 doctests failed sage t long /usr/lib64/python2.7/sitepackages/sage/schemes/generic/algebraic_scheme.py # 9 doctests failed sage t long /usr/lib64/python2.7/sitepackages/sage/schemes/curves/projective_curve.py # 15 doctests failed sage t long /usr/lib64/python2.7/sitepackages/sage/rings/polynomial/multi_polynomial_ideal.py # 70 doctests failed sage t long /usr/lib64/python2.7/sitepackages/sage/modular/modform_hecketriangle/abstract_ring.py # 7 doctests failed sage t long /usr/lib64/python2.7/sitepackages/sage/tests/french_book/mpoly.py # 14 doctests failed sage t long /usr/lib64/python2.7/sitepackages/sage/rings/polynomial/multi_polynomial_sequence.py # 6 doctests failed sage t long /usr/lib64/python2.7/sitepackages/sage/interfaces/singular.py # 2 doctests failed sage t long /usr/lib64/python2.7/sitepackages/sage/matrix/matrix_sparse.pyx # 3 doctests failed sage t long /usr/lib64/python2.7/sitepackages/sage/schemes/curves/affine_curve.py # 15 doctests failed sage t long /usr/lib64/python2.7/sitepackages/sage/algebras/commutative_dga.py # 366 doctests failed sage t long /usr/lib64/python2.7/sitepackages/sage/algebras/free_algebra.py # 10 doctests failed sage t long /usr/lib64/python2.7/sitepackages/sage/geometry/polyhedron/palp_database.py # 1 doctest failed sage t long /usr/lib64/python2.7/sitepackages/sage/rings/polynomial/multi_polynomial_libsingular.pyx # 2 doctests failed sage t long /usr/lib64/python2.7/sitepackages/sage/rings/polynomial/plural.pyx # 367 doctests failed sage t long /usr/lib64/python2.7/sitepackages/sage/libs/singular/function.pyx # 22 doctests failed sage t long /usr/lib64/python2.7/sitepackages/sage/schemes/projective/projective_space.py # 4 doctests failed sage t long /usr/lib64/python2.7/sitepackages/sage/libs/singular/groebner_strategy.pyx # 40 doctests failed
That one is ok
sage t long /usr/lib64/python2.7/sitepackages/sage/interfaces/singular.py ********************************************************************** File "/usr/lib64/python2.7/sitepackages/sage/interfaces/singular.py", line 1088, in sage.interfaces.singular.Singular.set_ring Failed example: singular.current_ring() Expected: polynomial ring, over a field, local/mixed ordering // characteristic : 7 // number of vars : 2 // block 1 : ordering ds // : names a b // block 2 : ordering C Got: polynomial ring, over a field, local ordering // characteristic : 7 // number of vars : 2 // block 1 : ordering ds // : names a b // block 2 : ordering C ********************************************************************** File "/usr/lib64/python2.7/sitepackages/sage/interfaces/singular.py", line 1997, in sage.interfaces.singular.SingularElement.set_ring Failed example: singular.current_ring() Expected: polynomial ring, over a field, local/mixed ordering // characteristic : 7 // number of vars : 2 // block 1 : ordering ds // : names a b // block 2 : ordering C Got: polynomial ring, over a field, local ordering // characteristic : 7 // number of vars : 2 // block 1 : ordering ds // : names a b // block 2 : ordering C ********************************************************************** 2 items had failures: 1 of 6 in sage.interfaces.singular.Singular.set_ring 1 of 6 in sage.interfaces.singular.SingularElement.set_ring
But a lot of problem seem to go back to libs/singular/function.pyx
sage t long /usr/lib64/python2.7/sitepackages/sage/libs/singular/function.pyx ********************************************************************** File "/usr/lib64/python2.7/sitepackages/sage/libs/singular/function.pyx", line 307, in sage.libs.singular.function.Resolution.__init__ Failed example: resolution = mres(M, 0) Exception raised: Traceback (most recent call last): File "/usr/lib64/python2.7/sitepackages/sage/doctest/forker.py", line 501, in _run self.compile_and_execute(example, compiler, test.globs) File "/usr/lib64/python2.7/sitepackages/sage/doctest/forker.py", line 864, in compile_and_execute exec(compiled, globs) File "<doctest sage.libs.singular.function.Resolution.__init__[6]>", line 1, in <module> resolution = mres(M, Integer(0)) File "sage/libs/singular/function.pyx", line 1323, in sage.libs.singular.function.SingularFunction.__call__ (/scratch2/portage/scimathematics/sage9999/work/sage9$ return call_function(self, args, ring, interruptible, attributes) File "sage/libs/singular/function.pyx", line 1500, in sage.libs.singular.function.call_function (/scratch2/portage/scimathematics/sage9999/work/sage9999/srcpyth$ with opt_ctx: # we are preserving the global options state here File "sage/libs/singular/function.pyx", line 1502, in sage.libs.singular.function.call_function (/scratch2/portage/scimathematics/sage9999/work/sage9999/srcpyth$ sig_on() File "src/cysignals/signals.pyx", line 108, in cysignals.signals.sig_raise_exception (build/src/cysignals/signals.c:1448) SignalError: Segmentation fault **********************************************************************
And more in the same style that go to the with opt_ctx: # we are preserving the global options state here
line before segfaulting. There are a few other original one but fixing the above would considerably reduce the number of failure all over the place.
Ideas?
comment:14 Changed 6 years ago by
Adding https://github.com/Singular/Sources/commit/861b7899904e3bbe3e8fe49fc0dd11ef9f5f238e to singular 4.0.3p4, I was able to compile sageongentoo with ntl10.1.0. A doc test run didn't reveal any failures from ntl. Since we are now supporting C++11 should we enable threads in ntl (this requires C++11)?
comment:15 Changed 6 years ago by
Am I understanding you correctly that we require a C++11 compiler? If so, then I think we should enable threads. If it is something we check at configuretime, then I would say we should extend that to NTL as well.
comment:16 Changed 6 years ago by
Commit:  d10b2e0f04e0f08fadee94a17849b4cb6040ba34 → f4fd671350d67eaf334a195b086a25777553c38b 

Branch pushed to git repo; I updated commit sha1. New commits:
4378994  Update singular to 4.0.3p4.

b191d91  Don't clean up args to singular function without a ring set.

657fac9  Different Singular printing for ordering.

eef0dc4  Use Singular p_WDegree for weighted degree.

10b99df  Remove unused Singular degree related stuff.

ff7c145  Touch Singular autotools files in spkgsrc.

147b12b  Merge branch 'singular4' into ntl10

3484f58  Update NTL to 10.1.0.

f4fd671  Patch Singular 4 to build against NTL 10+.

comment:17 Changed 6 years ago by
Dependencies:  → #21631 

Description:  modified (diff) 
Status:  needs_work → needs_review 
Summary:  Upate NTL to 10.0.0 → Upate NTL to 10.1.0 
Update to 10.1.0. The upstream tarball is no longer repackaged. I merged #21631, that was surely unnecessary, but it's done.
Enabling threads vould wait for another ticket? Or do we really think it will caue no problems?
comment:18 Changed 6 years ago by
Commit:  f4fd671350d67eaf334a195b086a25777553c38b → 4a75735aac6e5687403d57558cb5c8a3b62ca0f0 

Branch pushed to git repo; I updated commit sha1. New commits:
4a75735  Remove NTL spkgsrc script, we now use the upstream tarball.

comment:19 Changed 6 years ago by
Milestone:  sage7.4 → sage7.5 

comment:20 Changed 6 years ago by
I wouldn't think threads would create much problems in themselves. The main problem is that all ntl dependencies needs to use a c++11 compiler. It may actually deserve another ticket because we may have to touch the sage library itself on a few key files and that's better done separately. The other possible trouble is increased random failures in parallel doctesting because the machine is oversubscribed.
comment:21 Changed 6 years ago by
I would just try it and see what breaks. If it works, then we can include it in; if it requires significant work, then we cut that off as the starting point for a new ticket.
I can't say anything about possible increased random failures.
comment:23 Changed 6 years ago by
flint
doesn't default to c++11 for its ntl
interface. That can easily be fixed at the spkginstall level. But by default it fails. Forgot about flint
.
comment:24 Changed 6 years ago by
OK, anything that includes ntlwrap.h
needs to have a std=c++11
flag added to it. Let's save that for later.
comment:25 Changed 6 years ago by
Reviewers:  → François Bissey 

Status:  needs_review → positive_review 
I should have put positive review earlier, lgtm :)
comment:26 Changed 6 years ago by
Status:  positive_review → needs_work 

[ntl10.1.0] Running the test suite for ntl10.1.0... [ntl10.1.0] ./spkgcheck: line 3: cd: src/ntl/src: No such file or directory
comment:27 Changed 6 years ago by
Embarrassing... From what I can see in spkginstall
src/ntl/src
should be replaced by src/src
.
comment:28 Changed 6 years ago by
4.0.3p5 is out and has the ntl patch do we move along while fixing the tests?
comment:30 Changed 6 years ago by
Commit:  4a75735aac6e5687403d57558cb5c8a3b62ca0f0 → 091862e2f891dc4f5144ad6fedb66ad6bfafe2fd 

Branch pushed to git repo; I updated commit sha1. New commits:
091862e  Update NTL's spkgsrc script with new dir.

comment:31 Changed 6 years ago by
Status:  needs_work → positive_review 

Trivial change, I put this back to positive review. Sorry for missing that change before.
comment:32 Changed 6 years ago by
Branch:  public/ntl10 → 091862e2f891dc4f5144ad6fedb66ad6bfafe2fd 

Resolution:  → fixed 
Status:  positive_review → closed 
You want me to modify this to use the new version (which shouldn't need repacking)? Or would someone consider to first include this version for which the modifications are much more trivial?