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: | sage-7.5 |
Component: | packages: standard | Keywords: | |
Cc: | leif, fbissey, jdemeyer, vbraun | Merged in: | |
Authors: | Jean-Pierre 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 follow-up: 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 sage-devel 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/site-packages/sage/modular/modform_hecketriangle/graded_ring_element.py # 52 doctests failed sage -t --long /usr/lib64/python2.7/site-packages/sage/rings/asymptotic/asymptotics_multivariate_generating_functions.py # 29 doctests failed sage -t --long /usr/lib64/python2.7/site-packages/sage/modular/modform_hecketriangle/readme.py # 10 doctests failed sage -t --long /usr/lib64/python2.7/site-packages/sage/schemes/generic/algebraic_scheme.py # 9 doctests failed sage -t --long /usr/lib64/python2.7/site-packages/sage/schemes/curves/projective_curve.py # 15 doctests failed sage -t --long /usr/lib64/python2.7/site-packages/sage/rings/polynomial/multi_polynomial_ideal.py # 70 doctests failed sage -t --long /usr/lib64/python2.7/site-packages/sage/modular/modform_hecketriangle/abstract_ring.py # 7 doctests failed sage -t --long /usr/lib64/python2.7/site-packages/sage/tests/french_book/mpoly.py # 14 doctests failed sage -t --long /usr/lib64/python2.7/site-packages/sage/rings/polynomial/multi_polynomial_sequence.py # 6 doctests failed sage -t --long /usr/lib64/python2.7/site-packages/sage/interfaces/singular.py # 2 doctests failed sage -t --long /usr/lib64/python2.7/site-packages/sage/matrix/matrix_sparse.pyx # 3 doctests failed sage -t --long /usr/lib64/python2.7/site-packages/sage/schemes/curves/affine_curve.py # 15 doctests failed sage -t --long /usr/lib64/python2.7/site-packages/sage/algebras/commutative_dga.py # 366 doctests failed sage -t --long /usr/lib64/python2.7/site-packages/sage/algebras/free_algebra.py # 10 doctests failed sage -t --long /usr/lib64/python2.7/site-packages/sage/geometry/polyhedron/palp_database.py # 1 doctest failed sage -t --long /usr/lib64/python2.7/site-packages/sage/rings/polynomial/multi_polynomial_libsingular.pyx # 2 doctests failed sage -t --long /usr/lib64/python2.7/site-packages/sage/rings/polynomial/plural.pyx # 367 doctests failed sage -t --long /usr/lib64/python2.7/site-packages/sage/libs/singular/function.pyx # 22 doctests failed sage -t --long /usr/lib64/python2.7/site-packages/sage/schemes/projective/projective_space.py # 4 doctests failed sage -t --long /usr/lib64/python2.7/site-packages/sage/libs/singular/groebner_strategy.pyx # 40 doctests failed
That one is ok
sage -t --long /usr/lib64/python2.7/site-packages/sage/interfaces/singular.py ********************************************************************** File "/usr/lib64/python2.7/site-packages/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/site-packages/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/site-packages/sage/libs/singular/function.pyx ********************************************************************** File "/usr/lib64/python2.7/site-packages/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/site-packages/sage/doctest/forker.py", line 501, in _run self.compile_and_execute(example, compiler, test.globs) File "/usr/lib64/python2.7/site-packages/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/sci-mathematics/sage-9999/work/sage-9$ 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/sci-mathematics/sage-9999/work/sage-9999/src-pyth$ 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/sci-mathematics/sage-9999/work/sage-9999/src-pyth$ 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 sage-on-gentoo with ntl-10.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 configure-time, 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 spkg-src.
|
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 spkg-src script, we now use the upstream tarball.
|
comment:19 Changed 6 years ago by
Milestone: | sage-7.4 → sage-7.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 over-subscribed.
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 spkg-install 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 |
---|
[ntl-10.1.0] Running the test suite for ntl-10.1.0... [ntl-10.1.0] ./spkg-check: 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 spkg-install
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 spkg-src 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?