eclib: suppress / delete files left by doctests and check, fix building in parallel

[qed777]

From John Palmieri:

When I run doctests on the file ell_rational_field.py, I end up with a
small file called PRIMES in the current directory. This shouldn't
happen: running doctests shouldn't produce files in a non-temporary
directory.
[...]

This is a follow-up to #7575. Please see comment 24+ for some progress.

A new spkg is available at

• http://sage.math.washington.edu/home/mpatel/trac/8357/eclib-20080310.p10.spkg

It includes the PRIMES, 1, and make check fixes mentioned below and parallel build fixes (cf. #8306).

Note to release manager: Merge only the spkg.

Component: elliptic curves

Author: John Cremona, Mitesh Patel

Reviewer: Minh Van Nguyen

Merged: sage-4.3.4.alpha1

Issue created by migration from https://trac.sagemath.org/ticket/8357

[JohnCremona]

comment:1

The quickest quick fix would be just to comment out line 372 in src/procs/marith.cc (in eclib).

Anything else requires thought and testing, which I don't have time for i nthe next several days.

[qed777]

Author: John Cremona, Mitesh Patel

[qed777]

comment:3

There's still a timing(?) problem (log).

[qed777]

Attachment: trac_8357-eclib_makefiles.patch.gz

Tweak MAKEFILEs for parallel builds. eclib src repo.

[qed777]

comment:4

Replying to @qed777:

There's still a timing(?) problem (log).

I think the problem is that make sometimes attempts to build a target in progs before it's done building a requisite shared library, e.g., install_so. I've updated the "makefiles" patch and spkg with a fix that I'm testing now.

[JohnCremona]

comment:5

Replying to @qed777:

Replying to @qed777:

There's still a timing(?) problem (log).

I think the problem is that make sometimes attempts to build a target in progs before it's done building a requisite shared library, e.g., install_so. I've updated the "makefiles" patch and spkg with a fix that I'm testing now.

That sounds likely. We (I) should probably tidy this up; there are several useful executables which are built, but the only one actually used by and accessible from Sage after the build is mwrank. So i should change the targets so that the other progs are only built when doing a make check.

[qed777]

comment:6

Replying to @qed777:

[...] I've updated the "makefiles" patch and spkg with a fix that I'm testing now.

It seems to work and it survives a stress test on sage.math, e.g.,

export MAKE="make -j20"  # And NTL_PREFIX, etc.
make veryclean && make && make so && make veryclean && make && make so && [lots of reps] && ls

[JohnCremona]

comment:7

mpatel: if possible could you make he following additional edits to src/procs/Makefile (in eclib...p10):

diff -r 809e34b4c146 procs/Makefile
--- a/procs/Makefile	Sat Feb 27 15:42:31 2010 -0800
+++ b/procs/Makefile	Sun Feb 28 20:08:44 2010 +0000
@@ -105,7 +105,7 @@
 	gzip procs.tar
 
 check: $(TESTS)
-	rm -f PRIMES t
+	rm -f PRIMES t 1
 	./vectest1 < vectest.in >  t && diff t vectest.out
 	./vectest2 < vectest.in >  t && diff t vectest.out
 	./mattest1 < mattest.in >  t && diff t mattest.out
@@ -128,7 +128,7 @@
 	./rcubic < rcubic.in > t && diff t rcubic.out
 	./lcubic < lcubic.in > t && diff t lcubic.out
 	./tp2points < tp2points.in > t && diff t tp2points.out
-	rm -f PRIMES t
+	rm -f PRIMES t 1

It's another temp file which gets left behind, this time after "make check".

If that's too much hassle I'll do it next time I make changes to the eclib spkg, but it would be convenient to do it now.

[qed777]

Diff of spkg-install, SPKG.txt. eclib spkg repo.

[qed777]

Attachment: trac_8357-spkg.patch.gz

Attachment: trac_8357-suppress_PRIMES.patch.gz

Don't write PRIMES. Delete 1 after check. eclib src repo

[qed777]

comment:8

Replying to @JohnCremona:

If that's too much hassle I'll do it next time I make changes to the eclib spkg, but it would be convenient to do it now.

Done! I've updated spkg and two patches.

[qed777]

comment:9

I noticed a different problem with make check on t2:

make[3]: Entering directory `/scratch/mpatel/sage-4.3.0.1/spkg/build/eclib-20080310.p10/src/g0n'
rm -f PRIMES t
./modtest < modtest.in > t 2>/dev/null && diff t modtest.out 
./homtest < homtest.in > t && diff t homtest.out
./hecketest < hecketest.in > t 2>/dev/null && diff t hecketest.out 
./mhcount < mhcount.in > t && diff t  mhcount.out
rm -rf tmp_nf_dir
mkdir tmp_nf_dir
export NF_DIR=tmp_nf_dir && ./tmanin < tmanin.in > t 2>/dev/null && diff t tmanin.out
/bin/sh: NF_DIR=tmp_nf_dir: is not an identifier
make[3]: *** [check] Error 1
make[3]: Leaving directory `/scratch/mpatel/sage-4.3.0.1/spkg/build/eclib-20080310.p10/src/g0n'

I think we can fix this with, e.g.,

env NF_DIR=tmp_nf_dir ./tmanin < tmanin.in > t 2>/dev/null && diff t tmanin.out

[JohnCremona]

comment:10

I tested the current version of the spkg. (This does nto have the suggested changes to the handling of NF_DIR). All was well.

I did not make new changes (for the NF_DIR thing) since I thought that would be too confusing. But I'll be happy to check the next version of p10 in due course.

[JohnCremona]

comment:11

There is a simpler solution to the problem with "export NF_DIR". The code uses a default directory name for this, namely "newforms", which can be over-ridden by the use of the environment variable NF_DIR. For this test therefore we can replace the 2 lines "rm -rf tmp_nf_dir" by "rm -rf newforms" (strictly only the second one is needed) and remove all the starts of lines of the form "export NF_DIR=tmp_nf_dir && ".
(This is in src/g0n/Makefile).

Here's why I did not do this in the first place: by own private copy of this code has a newforms directory which contains 3.4G of data which took many many processor-years to compute. I do not want a simple "make check" to delete that so I put in this temporary (and non-default) tmp_nf_dir thing instead. But for the Sage distribution that is not needed.

I can attach a replacement Makefile here if that would be convenient, for yet another version of p10.

[qed777]

comment:12

Sure, that sounds good.

[JohnCremona]

replacement src/g0n/Makefile

[JohnCremona]

comment:13

Attachment: Makefile.gz

Replying to @qed777:

Sure, that sounds good.

I have attached the replacement Makefile -- could you update the spkg with it?

[qed777]

comment:14

Done! The package now builds in parallel and make check now works for me on t2 and sage.math.

[qed777]

Attachment: trac_8357-newforms_dir.patch.gz

Simplify g0n/Makefile. eclib src repo.

[sagetrac-mvngu]

comment:15

The updated eclib package eclib-20080310.p10.spkg looks good. It solves the unexpected annoyance introduced by #7575.

[sagetrac-mvngu]

Reviewer: Minh Van Nguyen

[mwhansen]

Merged: sage-4.3.4.alpha1