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:

GitHub link to the corresponding issue

Change History (32)

comment:1 Changed 6 years ago by jpflori

Description: modified (diff)
Status: newneeds_review

comment:2 Changed 6 years ago by jpflori

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?

comment:3 Changed 6 years ago by tscrim

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 Changed 6 years ago by 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).

comment:5 in reply to:  4 Changed 6 years ago by fbissey

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 fbissey

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 jpflori

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 jpflori

comment:9 Changed 6 years ago by fbissey

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 jdemeyer

Status: needs_reviewneeds_work

comment:11 Changed 6 years ago by fbissey

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:12 Changed 6 years ago by jpflori

No ideas... I'll have to try the new upstream tarball.

comment:13 Changed 6 years ago by jpflori

See #21631.

comment:14 Changed 6 years ago by fbissey

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 tscrim

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 git

Commit: d10b2e0f04e0f08fadee94a17849b4cb6040ba34f4fd671350d67eaf334a195b086a25777553c38b

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

4378994Update singular to 4.0.3p4.
b191d91Don't clean up args to singular function without a ring set.
657fac9Different Singular printing for ordering.
eef0dc4Use Singular p_WDegree for weighted degree.
10b99dfRemove unused Singular degree related stuff.
ff7c145Touch Singular autotools files in spkg-src.
147b12bMerge branch 'singular4' into ntl10
3484f58Update NTL to 10.1.0.
f4fd671Patch Singular 4 to build against NTL 10+.

comment:17 Changed 6 years ago by jpflori

Dependencies: #21631
Description: modified (diff)
Status: needs_workneeds_review
Summary: Upate NTL to 10.0.0Upate 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 git

Commit: f4fd671350d67eaf334a195b086a25777553c38b4a75735aac6e5687403d57558cb5c8a3b62ca0f0

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

4a75735Remove NTL spkg-src script, we now use the upstream tarball.

comment:19 Changed 6 years ago by jpflori

Milestone: sage-7.4sage-7.5

comment:20 Changed 6 years ago by fbissey

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 tscrim

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:22 Changed 6 years ago by fbissey

OK making an attempt.

comment:23 Changed 6 years ago by fbissey

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 fbissey

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 fbissey

Reviewers: François Bissey
Status: needs_reviewpositive_review

I should have put positive review earlier, lgtm :)

comment:26 Changed 6 years ago by vbraun

Status: positive_reviewneeds_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 fbissey

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 fbissey

4.0.3p5 is out and has the ntl patch do we move along while fixing the tests?

comment:29 Changed 6 years ago by fbissey

Wrong ticket...

comment:30 Changed 6 years ago by git

Commit: 4a75735aac6e5687403d57558cb5c8a3b62ca0f0091862e2f891dc4f5144ad6fedb66ad6bfafe2fd

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

091862eUpdate NTL's spkg-src script with new dir.

comment:31 Changed 6 years ago by jpflori

Status: needs_workpositive_review

Trivial change, I put this back to positive review. Sorry for missing that change before.

comment:32 Changed 6 years ago by vbraun

Branch: public/ntl10091862e2f891dc4f5144ad6fedb66ad6bfafe2fd
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.