#18845 closed enhancement (fixed)
Update NTL to 9.2.0
Reported by:  jpflori  Owned by:  

Priority:  major  Milestone:  sage6.8 
Component:  packages: standard  Keywords:  
Cc:  fbissey, jdemeyer  Merged in:  
Authors:  JeanPierre Flori  Reviewers:  Volker Braun 
Report Upstream:  N/A  Work issues:  
Branch:  4fd99b3 (Commits, GitHub, GitLab)  Commit:  
Dependencies:  #18866, #18867  Stopgaps: 
Description (last modified by )
Change History (56)
comment:1 Changed 6 years ago by
 Description modified (diff)
comment:2 Changed 6 years ago by
 Cc fbissey added
comment:3 Changed 6 years ago by
 Branch set to u/jpflori/ticket/18845
 Commit set to dd2285505edf4bf7b96283d7954de05333e232e6
I've updated three fuzzy/failing patching.
I've not touched the error callback patch which does not apply ass we have two solutions:
 update our patch,
 use the native NTL error callback mechanism, this can arguably be done elsewhere if the former solution is trivial.
There should still be trouble with flint, singular and friends. From what I read flint is fixed in git master. No clear idea about Singular. From what I read there is a fix upstream, but is it available for 3.x as we are stuck with that at the moment, or only for 4.x?
New commits:
dd22855  Update some NTL patches. WIP.

comment:4 Changed 6 years ago by
flint
will build against it but not singular
, I am not even sure singular4
will. I would need to check my inbox from when we looked at it in Gentoo with the guy that bumped ntl
to 9.x.
comment:5 followup: ↓ 6 Changed 6 years ago by
singular4
has a fix in master: https://github.com/Singular/Sources/commit/de688442dfe449992dc14a000bca0691ecc7e106 that may be back portable.
comment:6 in reply to: ↑ 5 Changed 6 years ago by
Replying to fbissey:
singular4
has a fix in master: https://github.com/Singular/Sources/commit/de688442dfe449992dc14a000bca0691ecc7e106 that may be back portable.
Yes, it seems it just applies as is (with offset).
comment:7 Changed 6 years ago by
 Commit changed from dd2285505edf4bf7b96283d7954de05333e232e6 to cedaaa41aa8b5bd123d8f399f60b553f1b7e440f
Branch pushed to git repo; I updated commit sha1. New commits:
cedaaa4  Fix NTL patches.

comment:8 Changed 6 years ago by
 Commit changed from cedaaa41aa8b5bd123d8f399f60b553f1b7e440f to 6611ff64f0490fde3d8e802c6cce92140e8476a3
Branch pushed to git repo; I updated commit sha1. New commits:
6611ff6  Use new style description for NTL patches.

comment:9 Changed 6 years ago by
 Commit changed from 6611ff64f0490fde3d8e802c6cce92140e8476a3 to ca9d43e58c82dcd6ecff92a68770e4e3c7e57b18
comment:10 Changed 6 years ago by
There is something wrong with my modifs to David's code. If someone could have a look, nothing jumps to me now...
comment:11 Changed 6 years ago by
My changes look pretty similar to the ones here:
except for using NTL::mulmod_t
instead of NTL::wide_double
.
comment:12 followup: ↓ 14 Changed 6 years ago by
No idea what the difference is I am afraid.
comment:13 Changed 6 years ago by
 Commit changed from ca9d43e58c82dcd6ecff92a68770e4e3c7e57b18 to 79246f0d651207b38ec0333d2a499dc54cf8a5de
Branch pushed to git repo; I updated commit sha1. New commits:
79246f0  Further fixes for NTL update.

comment:14 in reply to: ↑ 12 Changed 6 years ago by
Replying to fbissey:
No idea what the difference is I am afraid.
Could you test this on a usual arch? Currently, I only have Sage installed on a POWER7...
comment:15 Changed 6 years ago by
I'll have a look on my mac.
comment:16 Changed 6 years ago by
Taking longer than expected I wasn't completely up to date to beta7 (was on beta5). Will let you know when it is all done.
comment:17 Changed 6 years ago by
OK, I almost fell like I should have done a new build from scratch.
Mirage:sage6.8.beta5 fbissey$ ./sage t long warnlong 34.4 src/sage/rings/arith.py Running doctests with ID 20150707172618c6eb7fbc. Git branch: ntl92 Using optional=gcc,mpir,python2,sage,scons Doctesting 1 file. sage t long warnlong 34.4 src/sage/rings/arith.py ********************************************************************** File "src/sage/rings/arith.py", line 294, in sage.rings.arith.bernoulli Failed example: union([len(union(x))==1 for x in vals]) # long time (depends on previous line) Expected: [True] Got: [False, True] ********************************************************************** File "src/sage/rings/arith.py", line 299, in sage.rings.arith.bernoulli Failed example: union([len(union(x))==1 for x in vals]) # long time (depends on previous line) Expected: [True] Got: [False, True] ********************************************************************** 1 item had failures: 2 of 16 in sage.rings.arith.bernoulli [785 tests, 2 failures, 51.51 s]  sage t long warnlong 34.4 src/sage/rings/arith.py # 2 doctests failed  Total time for all tests: 51.7 seconds cpu time: 38.6 seconds cumulative wall time: 51.5 seconds Mirage:sage6.8.beta5 fbissey$ ./sage t long warnlong 34.4 src/sage/rings/bernmm.pyx Running doctests with ID 20150707190238d29775f0. Git branch: ntl92 Using optional=gcc,mpir,python2,sage,scons Doctesting 1 file. sage t long warnlong 34.4 src/sage/rings/bernmm.pyx ********************************************************************** File "src/sage/rings/bernmm.pyx", line 68, in sage.rings.bernmm.bernmm_bern_rat Failed example: lst1 == lst2 Expected: True Got: False ********************************************************************** File "src/sage/rings/bernmm.pyx", line 70, in sage.rings.bernmm.bernmm_bern_rat Failed example: [ Zmod(101)(t) for t in lst1 ] Expected: [77, 72, 89, 98, 86] Got: [99, 0, 44, 42, 57] ********************************************************************** 1 item had failures: 2 of 13 in sage.rings.bernmm.bernmm_bern_rat [23 tests, 2 failures, 0.79 s]  sage t long warnlong 34.4 src/sage/rings/bernmm.pyx # 2 doctests failed  Total time for all tests: 0.8 seconds cpu time: 1.2 seconds cumulative wall time: 0.8 seconds Mirage:sage6.8.beta5 fbissey$ ./sage t long warnlong 34.4 src/doc/en/thematic_tutorials/explicit_methods_in_number_theory/birds_other.rst Running doctests with ID 2015070719025871d664dd. Git branch: ntl92 Using optional=gcc,mpir,python2,sage,scons Doctesting 1 file. sage t long warnlong 34.4 src/doc/en/thematic_tutorials/explicit_methods_in_number_theory/birds_other.rst ********************************************************************** File "src/doc/en/thematic_tutorials/explicit_methods_in_number_theory/birds_other.rst", line 212, in doc.en.thematic_tutorials.explicit_methods_in_number_theory.birds_other Warning, slow doctest: w2 = bernoulli(100000, algorithm='pari') # long time (6s on sage.math, 2011) Test ran for 55.38 s ********************************************************************** File "src/doc/en/thematic_tutorials/explicit_methods_in_number_theory/birds_other.rst", line 213, in doc.en.thematic_tutorials.explicit_methods_in_number_theory.birds_other Failed example: w1 == w2 # long time Expected: True Got: False ********************************************************************** 1 item had failures: 1 of 42 in doc.en.thematic_tutorials.explicit_methods_in_number_theory.birds_other [34 tests, 1 failure, 62.40 s]  sage t long warnlong 34.4 src/doc/en/thematic_tutorials/explicit_methods_in_number_theory/birds_other.rst # 1 doctest failed  Total time for all tests: 62.4 seconds cpu time: 67.9 seconds cumulative wall time: 62.4 seconds
comment:18 Changed 6 years ago by
 Cc jdemeyer added
You get the same errors as I do.
And I cannot get NTL to build with NTL_LEGACY_SP_MULMOD=on
to check that the old double
implem is still fine.
Build fails with #if with no expression
just as in #3486 or https://groups.google.com/forum/#!topic/sagedevel/4r8HrQVOSe4
comment:19 Changed 6 years ago by
For info, the corresponding part of NTL doc is at:
comment:20 Changed 6 years ago by
My inability to try NTL_LEGACY_SP_MULMOD
is caused by the fact that InitSettings
does not build when it is set because of the typo:
static inline long MulMod2(long a, long b, long n, wide_double bninv) { return MulMod2_legacy(a, b, n, ninv); }
comment:21 followup: ↓ 23 Changed 6 years ago by
Upstream bug then.
comment:22 Changed 6 years ago by
 Commit changed from 79246f0d651207b38ec0333d2a499dc54cf8a5de to 3b1f14a3976e375ee23271a780b4e54e057549ec
Branch pushed to git repo; I updated commit sha1. New commits:
3b1f14a  Fix typo in sp_arith.h.

comment:23 in reply to: ↑ 21 ; followup: ↓ 25 Changed 6 years ago by
Replying to fbissey:
Upstream bug then.
Sure, I just sent a report to Victor Shoup.
Note that it does not explain the failure in bernmm
code with the new sp implem in (new) NTL.
It will just allow us to check that bernmm
works as expected with the old sp implem in (new) NTL.
comment:24 Changed 6 years ago by
And it works just fine.
You can try NTL built with the legacy sp implem at u/jpflori/ticket/18845legacy
.
comment:25 in reply to: ↑ 23 Changed 6 years ago by
comment:26 followup: ↓ 30 Changed 6 years ago by
Do you think you can fix the bernmm
problem? If not, using the old the "legacy" implementation is better than not merging this ticket.
comment:27 Changed 6 years ago by
If this is true:
NTL provides its own error callback framework
we should use it.
comment:28 followup: ↓ 34 Changed 6 years ago by
OK, we still need the patch to the callback, but not for the reason stated. So, please replace
NTL provides its own error callback framework, but it is simpler than the one provided by this patch: there is no context object.
by
NTL provides its own error callback framework, but it's not suitable for Sage since we don't want to see the error message printed, we want the error message to be part of the callback function.
comment:29 followup: ↓ 31 Changed 6 years ago by
I just sent an email to Shoup about the callback function.
comment:30 in reply to: ↑ 26 Changed 6 years ago by
Replying to jdemeyer:
Do you think you can fix the
bernmm
problem? If not, using the old the "legacy" implementation is better than not merging this ticket.
I'd say not quickly and I'm away for one week starting tonight.
I really have no idea why it does not work with the current patch.
The only way I see to understand this is to print everything bern_modp does to find the problem.
A small couple of problematic values is k = 128
and p = 7
(or another small prime).
So unless you see some obvious mistake in what I did, we can postpone the use of the new (and recommended upstream) sp implem for later.
We'll just need to remove one line in spkginstall
.
comment:31 in reply to: ↑ 29 Changed 6 years ago by
Replying to jdemeyer:
I just sent an email to Shoup about the callback function.
Thanks, I already mentioned that twice during our exchanges with Shoup but never really explained to him the difference between our and his error callback.
If by any chance you know why the C++ new
patch for Singular is needed, I'd be happy you tell Shoup and I.
comment:32 Changed 6 years ago by
Also note that NTL now provides exception handling though I've not activated it here.
comment:33 Changed 6 years ago by
 Commit changed from 3b1f14a3976e375ee23271a780b4e54e057549ec to a1b327a4b22269f79b237a1a936a5abde1431712
comment:34 in reply to: ↑ 28 Changed 6 years ago by
Replying to jdemeyer:
OK, we still need the patch to the callback, but not for the reason stated. So, please replace
NTL provides its own error callback framework, but it is simpler than the one provided by this patch: there is no context object.by
NTL provides its own error callback framework, but it's not suitable for Sage since we don't want to see the error message printed, we want the error message to be part of the callback function.
Done.
I've also merged the changes from the legacy branch.
comment:35 Changed 6 years ago by
 Commit changed from a1b327a4b22269f79b237a1a936a5abde1431712 to db8cf9977d8f5827371b128249f22348839872a4
Branch pushed to git repo; I updated commit sha1. New commits:
db8cf99  No c++11 for NTL without threads and exceptions.

comment:36 Changed 6 years ago by
Shoup answered that he will update the callback function such that it will be good enough for Sage.
comment:37 Changed 6 years ago by
 Status changed from new to needs_review
comment:38 Changed 6 years ago by
 Status changed from needs_review to needs_work
Traceback (most recent call last): File "/usr/local/src/sageconfig/src/doc/common/builder.py", line 1619, in <module> import sage.all File "/usr/local/src/sageconfig/local/lib/python2.7/sitepackages/sage/all.py", line 103, in <module> import sage.symbolic.pynac ImportError: /usr/local/src/sageconfig/local/lib/python2.7/sitepackages/sage/symbolic/pynac.so: undefined symbol: _ZN3NTL5ErrorEPKc
comment:39 followup: ↓ 41 Changed 6 years ago by
It's a dependency problem: that module should depend on NTL, but it doesn't.
comment:40 Changed 6 years ago by
Strange:
[sage.git]$ ./sage ┌────────────────────────────────────────────────────────────────────┐ │ SageMath Version 6.8.beta7, Release Date: 20150702 │ │ Type "notebook()" for the browserbased notebook interface. │ │ Type "help()" for help. │ └────────────────────────────────────────────────────────────────────┘ ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ ┃ Warning: this is a prerelease version, and it may be unstable. ┃ ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ sage: import sage.symbolic.pynac sage:
and
[sage.git]$ nm local/lib/python/sitepackages/sage/symbolic/pynac.so grep NTL 000000000000c610 t 00000058.plt_call._ZN3NTL13TerminalErrorEPKc+0 000000000000d088 t 00000058.plt_call._ZN3NTL14BlockConstructEPNS_4ZZ_pEl+0 000000000000ca20 t 00000058.plt_call._ZN3NTL3VecINS_4ZZ_pEE10AllocateToEl+0 000000000000c7a0 t 00000058.plt_call._ZN3NTL3VecINS_5ZZ_pEEE10AllocateToEl+0 000000000000d074 t 00000058.plt_call._ZN3NTL5ZZ_pX9normalizeEv+0 000000000000c55c t 00000058.plt_call._ZN3NTL6ZZ_pEX9normalizeEv+0 000000000000c764 t 00000058.plt_call._ZNK3NTL11ZZ_pContext7restoreEv+0 0000000000078cf0 D _Z19ZZ_pEX_conv_modulusRN3NTL6ZZ_pEXERKS0_RKNS_11ZZ_pContextE U _ZN3NTL13TerminalErrorEPKc U _ZN3NTL14BlockConstructEPNS_4ZZ_pEl 0000000000078cd0 W _ZN3NTL3VecINS_4ZZ_pEE10AllocateToEl 0000000000078ce0 W _ZN3NTL3VecINS_5ZZ_pEEE10AllocateToEl U _ZN3NTL5ZZ_pX9normalizeEv U _ZN3NTL6ZZ_pEX9normalizeEv U _ZN3NTL8ZZ_pInfoE U _ZNK3NTL11ZZ_pContext7restoreEv
Plain Error
does not exist anymore in NTL 9.x.
comment:41 in reply to: ↑ 39 Changed 6 years ago by
Replying to jdemeyer:
It's a dependency problem: that module should depend on NTL, but it doesn't.
I see. Can you fix this or should I do it?
comment:42 Changed 6 years ago by
Hmm, it's strange that this module isn't actually linked against ntl
...
comment:43 Changed 6 years ago by
I'm lost here... regardless of this ticket, the module sage.symbolic.pynac
is not linked against NTL, but it does contain those NTL symbols.
comment:44 Changed 6 years ago by
C++ templates black magic ? :)
Actually I had a quick look at the pyx file and found nothing about NTL...
comment:45 Changed 6 years ago by
Had a quick look earlier this morning, the only way ntl
can get pulled is by a chain of include files that are not obvious to follow. sage.symbolic.pynac
import some stuff from sage.rings
and I think something in rings.pxd
import some ntl
stuff eventually. I will look for where precise NTL
symbols are used next.
comment:46 Changed 6 years ago by
I'll be in a hidden valley for one week and though I might reply here, I won't have any ssh connection and any chance to fix this, so feel free to close the ticket without my help!
comment:47 Changed 6 years ago by
It seems that the module is really underlinked:
$ ./sage python Python 2.7.9 (default, Jul 5 2015, 21:49:10) [GCC 4.8.4] on linux2 Type "help", "copyright", "credits" or "license" for more information. >>> import sage.symbolic.pynac Traceback (most recent call last): File "<stdin>", line 1, in <module> ImportError: /usr/local/src/sageconfig/local/lib/python2.7/sitepackages/sage/symbolic/pynac.so: undefined symbol: _ZN3NTL8ZZ_pInfoE
The only reason there is no error in Sage is that NTL was imported before (by some other module imported earlier).
comment:48 Changed 6 years ago by
 Dependencies set to #18866
There are many modules with the same linking problem. I think it's pointless to fix that problem without cleaning up the whole NTL interface (which is a big mess).
That leaves the problem of the dependency checking: I opened #18866 for that.
comment:49 Changed 6 years ago by
 Branch changed from u/jpflori/ticket/18845 to u/jdemeyer/ticket/18845
comment:50 Changed 6 years ago by
 Commit changed from db8cf9977d8f5827371b128249f22348839872a4 to 50a36992701a707aaa319bcf447a833b4ea600df
comment:51 Changed 6 years ago by
 Dependencies changed from #18866 to #18866, #18867
comment:52 Changed 6 years ago by
 Commit changed from 50a36992701a707aaa319bcf447a833b4ea600df to 4fd99b370fca63dd04213519f614df7721314fba
comment:53 Changed 6 years ago by
 Status changed from needs_work to needs_review
All doctests pass (but I haven't actually reviewed the changes)
comment:54 Changed 6 years ago by
 Reviewers set to Volker Braun
 Status changed from needs_review to positive_review
comment:55 Changed 6 years ago by
 Branch changed from u/jdemeyer/ticket/18845 to 4fd99b370fca63dd04213519f614df7721314fba
 Resolution set to fixed
 Status changed from positive_review to closed
comment:56 Changed 6 years ago by
 Commit 4fd99b370fca63dd04213519f614df7721314fba deleted
Followup: #18875
CCing sog folk as they already worked hard to include an uptodate NTL there.