interface.h wrongly claims NTL_ALL is the default

[jdemeyer]

The third point from the following comments in interface.h is wrong:

//  The following macro switches can be used: it is best to define them
//  in Makefiles rather than changing this file.
//  1. NTL_INTS     Use NTL for bigints (no bigrationals)
//  2. NTL_ALL      Use NTL also for bigfloats (RR) and bigcomplexes (CC)
//  3. (neither)    Defaults to NTL_ALL

Not defining any of these macros is not the same as defining NTL_ALL. In reality, it results in a compiler error.

[JohnCremona]

That sounds plausible. For quite a while I have only really supported the NTL_ALL option anyway, but there is code which is revealed when NTL_INTS is set (and nto NTL_ALL) in which standard precision version of certain functions are implemented. The idea is that one day both the standard and multi-precision versions will be available side-by-side as alternaitves rather than having to switch all at once at compile time. But that has not been done yet.

Until then: the NTL_INTS flag is redundant so should always be set, or (better) written out of the code. Then setting NTL_ALL could becomes a real option. But note that the autotools conversion did not allow for this anyway: "-DNTL_ALL" is hard-wired into the Makefile templates (Makefile.am files). So the code which would be compiled and run when NTL_ALL is not set has not been tested or used since I started the repackaging for Sage in 2007.

[JohnCremona]

OK, so Step 1 in this is to remove the NTL_INTS flag so that it does at least look as if there are precisely 2 options (with NTL_ALL set and without). Step 2 would be to get the code working again without NTL_ALL set. (One problem here is that the output will look different so automatic testing will be a small nuisance to fix, but doable). Then Step 3 will allow the Configure script to decide whether ot not to set NTL_ALL.

I will start to work on this.

[JohnCremona]

I have started a branch called NTL. The first commit there already wrotes NTL_INTS out of the script (and also updates the version number to 2016-01-01!)

[jdemeyer]

Note that the application I have in mind is the use of eclib as library in Sage, not so much compiling eclib itself.

[JohnCremona]

Yes -- and in that context you would need to do something rather artificial to not have NTL_ALL defined, which is why it has never been a problem. But I needed an excuse to tidy this up anyway. I have already got to the point where the code compiles and runs with NTL_ALL not set but a few tests have different output owing to the lower precision.

[JohnCremona]

... and some need separate test input files since they start by prompting for the precision to be input.

[jdemeyer]

in that context you would need to do something rather artificial to not have NTL_ALL defined,

???

In fact, the opposite is true: we need to artificially define NTL_ALL, otherwise the build breaks.

[JohnCremona]

The current master branch (tag: v20160101) implements all that was mentioned above as Steps 1, 2, 3.
Documentation has been updated accordingly, and I think is now accurate.

It is possible to include --disable-mpfp on the command line in which case the code is compiled using standard C doubles instead of NTL RR. This is not recommended (and is not the default) but the "make check|" tests do still pass (in some cases the expected output differs, simply because floating pint numbers output have different precision); it does serve to show that the non-multi-precision floating point code still works, so that one day tha code and the mpfp code will coexist instead of having to switch between them at configure time.

The library revision number has been increased (no changes to interface with default configure options). The name of te shared library built is now libec.so.2.0.3.

Once Jeroen is happy, this issue can be closed.

[jdemeyer]

I'd rather wait for http://trac.sagemath.org/ticket/19818 to be merged first. Remind me to look into this again then.

[JohnCremona]

OK. There is certainly no hurry getting this new release into Sage since it is functionally identical.

[JohnCremona]

Jeroen, on 3 Jan I said "when Jeroen is happy this can be closed" and you asked to wait until Sage trac #19818 was merged, which it now is. Let me know when you are happy and I'll close this issue.

[JohnCremona]

Quoting from discussion at https://trac.sagemath.org/ticket/22077:

jdemeyer:

-DNTL_ALL should not be needed. That was precisely the point that I was trying to make: eclib should not force other packages (like Sage) to use certain compiler flags. The NTL_ALL macro should be considered an implementation detail of eclib and it should not influence other software built against eclib.

JohnCremona:

I understand your point, but it is more than just an implementation detail, as it affects the interface. When compiled with the NTL_ALL option, functions which compute real or complex values (e.g. periods of elliptic curves) do so to arbitrary precision, returning values with type RR (NTL reals) for example, while without that option the same named function will return a C double.

I do understand that this can be viewed as bad design on my part and not a feature which a professional library should have. eclib has undergone a lot of transition since it was just "my code" (until 2007) so that by now it is almost respectable. One of the remaining issues is this one: users should not have to decide at build time whether they want to use doubles or arbitrary precision for a large collection of functions; the library should provide both and allow the uers to use whichever is most appropriate in the circumstances. In some cases, because of overloading, the same function name can be re-used; but in others different names would be necessary (e.g. for periods of elliptic curves, where the input is the same exact data whether ones wants low or high precision). It will happen one day.

[JohnCremona]

It would be nice to close this -- perhaps after v20180710 is in Sage (which may require some attention since the include files are now better organised).

[JohnCremona]

@jdemeyer This switch is now configured so that the compiler flag NTL_ALL is set if and only if the configure options --disable-mpfp is not set. Sage does not use this configure flag when building eclib, so has NTL_ALL set when building eclib.

I still see mention of NTL_ALL in Sage code: in sage.libs.eclib.init.pxd there is the line

distutils: extra_compile_args = -DNTL_ALL

which I suppose is there since when cython uses eclib's header files that flag needs to be set when cython files are compiled.

I can understand that being seen as undesirable and will think about changing this (which will be simpler than it would have been 3 years ago).

[JohnCremona]

PR #47 (which is already merged into master) does what I suggested above. By default no flags are needed either to build or used the library. See the commit message at PR #47 . I will wait a hort time for more comments and then close this.

[jdemeyer]

I'll need to test in Sage whether this is really fixed.

[JohnCremona]

Fair enough. I'll make a new Sage package, maybe next week since I have talk to write now.

[jdemeyer]

Works for me!