#8059 closed enhancement (fixed)
Update Singular spkg to release 3-1-1-4
Reported by: | Martin Albrecht | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-4.5.3 |
Component: | packages: standard | Keywords: | |
Cc: | PolyBoRi team, David Kirkby, Alex Ghitza, Jaap Spies, François Bissey, Alexander Dreyer | Merged in: | sage-4.5.3.alpha1 |
Authors: | Martin Albrecht | Reviewers: | David Kirkby, Simon King, William Stein, John Palmieri |
Report Upstream: | Reported upstream. Little or no feedback. | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
The Singular team accepted most of our patches upstream. They are in the 3-1-0-9 release, which also is a first step to make things easier for library developers.
How to apply the patches to sage-4.5.3.alpha0:
- Install the new Singular spkg: http://sage.math.washington.edu/home/mpatel/trac/8059/singular-3-1-1-4.p0.spkg
- Apply singular-3-1-1-4.2.patch to
devel/sage
Attachments (8)
Change History (142)
comment:1 Changed 13 years ago by
Cc: | PolyBoRi team David Kirkby Alex Ghitza added |
---|
Changed 13 years ago by
Attachment: | singular_3-0-1-9.patch added |
---|
comment:2 Changed 13 years ago by
Status: | new → needs_review |
---|
comment:3 Changed 13 years ago by
Doctests on sage.math:
The only failure is in gen.pyx. I don't see how this can be caused by my changes.
********************************************************************** File "/mnt/usb1/scratch/malb/sage-4.3.1/devel/sage/sage/libs/pari/gen.pyx", line 2950: sage: pari(sqrt(2)).frac() Exception raised: Traceback (most recent call last): File "/mnt/usb1/scratch/malb/sage-4.3.1/local/bin/ncadoctest.py", line 1231, in run_one_test self.run_one_example(test, example, filename, compileflags) File "/mnt/usb1/scratch/malb/sage-4.3.1/local/bin/sagedoctest.py", line 38, in run_one_example OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags) File "/mnt/usb1/scratch/malb/sage-4.3.1/local/bin/ncadoctest.py", line 1172, in run_one_example compileflags, 1) in test.globs File "<doctest __main__.example_77[3]>", line 1, in <module> pari(sqrt(Integer(2))).frac()###line 2950: sage: pari(sqrt(2)).frac() File "/mnt/usb1/scratch/malb/sage-4.3.1/local/lib/python/site-packages/sage/functions/other.py", line 812, in sqrt return x.sqrt(*args, **kwds) File "integer.pyx", line 4440, in sage.rings.integer.Integer.sqrt (sage/rings/integer.c:25864) File "/mnt/usb1/scratch/malb/sage-4.3.1/local/lib/python/site-packages/sage/functions/other.py", line 755, in _do_sqrt z = SR(x) ** one_half File "expression.pyx", line 2218, in sage.symbolic.expression.Expression.__pow__ (sage/symbolic/expression.cpp:11255) File "integer.pyx", line 1603, in sage.rings.integer.Integer.__pow__ (sage/rings/integer.c:11625) File "rational.pyx", line 2082, in sage.rings.rational.Rational.__pow__ (sage/rings/rational.c:15560) ImportError: /mnt/usb1/scratch/malb/sage-4.3.1/local/lib/python/site-packages/sage/symbolic/power_helper.so: failed to map segment from shared object: Cannot allocate memory ********************************************************************** File "/mnt/usb1/scratch/malb/sage-4.3.1/devel/sage/sage/libs/pari/gen.pyx", line 2977: sage: pari(sqrt(-2)).imag() Exception raised: Traceback (most recent call last): File "/mnt/usb1/scratch/malb/sage-4.3.1/local/bin/ncadoctest.py", line 1231, in run_one_test self.run_one_example(test, example, filename, compileflags) File "/mnt/usb1/scratch/malb/sage-4.3.1/local/bin/sagedoctest.py", line 38, in run_one_example OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags) File "/mnt/usb1/scratch/malb/sage-4.3.1/local/bin/ncadoctest.py", line 1172, in run_one_example compileflags, 1) in test.globs File "<doctest __main__.example_78[3]>", line 1, in <module> pari(sqrt(-Integer(2))).imag()###line 2977: sage: pari(sqrt(-2)).imag() File "/mnt/usb1/scratch/malb/sage-4.3.1/local/lib/python/site-packages/sage/functions/other.py", line 812, in sqrt return x.sqrt(*args, **kwds) File "integer.pyx", line 4424, in sage.rings.integer.Integer.sqrt (sage/rings/integer.c:25709) File "/mnt/usb1/scratch/malb/sage-4.3.1/local/lib/python/site-packages/sage/functions/other.py", line 755, in _do_sqrt z = SR(x) ** one_half File "expression.pyx", line 2218, in sage.symbolic.expression.Expression.__pow__ (sage/symbolic/expression.cpp:11255) File "integer.pyx", line 1603, in sage.rings.integer.Integer.__pow__ (sage/rings/integer.c:11625) File "rational.pyx", line 2082, in sage.rings.rational.Rational.__pow__ (sage/rings/rational.c:15560) ImportError: /mnt/usb1/scratch/malb/sage-4.3.1/local/lib/python/site-packages/sage/symbolic/power_helper.so: failed to map segment from shared object: Cannot allocate memory
comment:4 Changed 13 years ago by
Also, the same doctest passes for me locally, so I will assume for now that this has nothing to do with my patch (famous last words, I know).
comment:5 Changed 13 years ago by
Cc: | Jaap Spies added |
---|
comment:6 follow-up: 7 Changed 13 years ago by
I'm testing this on a couple of machines and will report.
I have two questions about the spkg:
- Should we remove dist/, see #5903 ?
- In src/, should we remove doc/ and emacs/ ? It's really just a question of making the spkg smaller by about 10%, since AFAIK these won't get used from Sage. We routinely do this with other spkgs.
I'm happy to do this and post a new spkg, but I wanted to see what Martin thought about it.
comment:7 follow-up: 11 Changed 13 years ago by
Replying to AlexGhitza:
I have two questions about the spkg:
- Should we remove dist/, see #5903 ?
Yes, that sounds reasonable.
- In src/, should we remove doc/ and emacs/ ? It's really just a question of making the spkg smaller by about 10%, since AFAIK these won't get used from Sage. We routinely do this with other spkgs.
I just checked and it still builds with these two directories removed.
Thus, I've updated the SPKG accordingly. Same place, same name.
comment:8 follow-up: 9 Changed 13 years ago by
I did not have any time to test it. But most changes introduce passing the ring explicitly to Singular. This avoid current ring problems (which can be quite annoying and hard to debug).
nlGetNumerator seems a new addition in Singular (maybe, a special wish of Martin?).
Cheers, Michael
comment:9 Changed 13 years ago by
Replying to PolyBoRi:
I did not have any time to test it. But most changes introduce passing the ring explicitly to Singular. This avoid current ring problems (which can be quite annoying and hard to debug).
Yep, that's a great direction things are developing.
nlGetNumerator seems a new addition in Singular (maybe, a special wish of Martin?).
It's just nlGetNom
renamed
comment:10 follow-up: 12 Changed 13 years ago by
Then, what I can still comment are wild casts like
<napoly*>pNext(<poly*>z)
But they are also okay as napoly is just a typedef for polyrec*. So, from reading Malbs code, it looks good. Still someone needs to test it and check it for Sages formal criteria
comment:11 follow-up: 13 Changed 13 years ago by
Replying to malb:
I just checked and it still builds with these two directories removed.
Thus, I've updated the SPKG accordingly. Same place, same name.
Unless I am looking at the wrong SPKG, you forgot to take dist/
out of version control:
[ghitza@cartan singular-3-1-0-9-20100125]$ hg status ! dist/debian/changelog ! dist/debian/compat ! dist/debian/control ! dist/debian/control.in ! dist/debian/copyright ! dist/debian/libsingular-dev.install ! dist/debian/libsingular0.install ! dist/debian/patches/change-library-search-path.patch ! dist/debian/patches/sage-bugfixes.patch ! dist/debian/patches/series ! dist/debian/rules ! dist/debian/singular.install
Apart from this, I'm happy.
comment:12 Changed 13 years ago by
Replying to PolyBoRi:
Then, what I can still comment are wild casts like But they are also okay as napoly is just a typedef for polyrec*.
There used to be a naIter
function which is gone in 3-1-0-9 and Singular internally uses pNext()
as well. Thus I feel my casting is justified :)
comment:13 Changed 13 years ago by
Unless I am looking at the wrong SPKG, you forgot to take
dist/
out of version control:
fixed. Is that a positive review then?
comment:15 follow-up: 16 Changed 13 years ago by
As is the spkg failed to build on Open Solaris x64 with SAGE64=yes.
g++ -c cf_factor.cc -w -fno-implicit-templates -I. -I. -I/export/home/jaap/Downloads/sage-4.3.2.alpha0/local/include -DHAVE_CONFIG_H -I/export/home/jaap/Downloads/sage-4.3.2.alpha0/local/include -I/export/home/jaap/Downloads/sage-4.3.2.alpha0/local/include -I/export/home/jaap/Downloads/sage-4.3.2.alpha0/local/include -O3 -g -fPIC -o cf_factor.o In file included from /export/home/jaap/Downloads/sage-4.3.2.alpha0/local/include/NTL/vec_ZZ.h:5, from /export/home/jaap/Downloads/sage-4.3.2.alpha0/local/include/NTL/ZZX.h:5, from /export/home/jaap/Downloads/sage-4.3.2.alpha0/local/include/NTL/ZZXFactoring.h:5, from NTLconvert.h:23, from cf_factor.cc:33: /export/home/jaap/Downloads/sage-4.3.2.alpha0/local/include/NTL/ZZ.h: In function ‘long int NTL::MulModPrecon(long int, long int, long int, long unsigned int)’: /export/home/jaap/Downloads/sage-4.3.2.alpha0/local/include/NTL/ZZ.h:1795: error: ‘MulHiUL’ was not declared in this scope make[2]: *** [cf_factor.o] Error 1 make[2]: Leaving directory `/export/home/jaap/Downloads/sage-4.3.2.alpha0/spkg/build/singular-3-1-0-9-20100125/src/factory' make[1]: *** [install] Error 1 make[1]: Leaving directory `/export/home/jaap/Downloads/sage-4.3.2.alpha0/spkg/build/singular-3-1-0-9-20100125/src' make: *** [/export/home/jaap/Downloads/sage-4.3.2.alpha0/local/bin/Singular-3-1-0] Error 2 Unable to build Singular. real 0m24.240s user 0m12.235s sys 0m9.934s sage: An error occurred while installing singular-3-1-0-9-20100125
Maybe it will install a some FLAGS are set globally (-m64) in this case).
As Open Solaris is not a supported platform I'll not interfere with the positive review.
Jaap
comment:16 follow-up: 18 Changed 13 years ago by
Jaap,
Did the previous Singular spkg build properly on Open Solaris? If so, I wonder if it's just a problem in spkg-install, or a more serious issue with Singular itself.
comment:17 Changed 13 years ago by
Owner: | tbd deleted |
---|
If CFLAGS and friends are set tight. Building on Open Solaris fails on
gcc -g -pg -O3 -I. -I/export/home/jaap/Downloads/sage-4.3.2.alpha0/local/include -O2 -g -m64 -I/export/home/jaap/Downloads/sage-4.3.2.alpha0/local/include -DHAVE_CONFIG_H -c omBinPage.c -o omBinPage.op /var/tmp//ccciayz2.s: Assembler messages: /var/tmp//ccciayz2.s:31: Error: junk `@' after expression /var/tmp//ccciayz2.s:110: Error: junk `@' after expression /var/tmp//ccciayz2.s:183: Error: junk `@' after expression /var/tmp//ccciayz2.s:270: Error: junk `@' after expression /var/tmp//ccciayz2.s:477: Error: junk `@' after expression /var/tmp//ccciayz2.s:805: Error: junk `@' after expression /var/tmp//ccciayz2.s:1240: Error: junk `@' after expression /var/tmp//ccciayz2.s:1718: Error: junk `@' after expression /var/tmp//ccciayz2.s:2041: Error: junk `@' after expression make[2]: *** [omBinPage.op] Error 1 make[2]: Leaving directory `/export/home/jaap/Downloads/sage-4.3.2.alpha0/spkg/build/singular-3-1-0-9-20100125/src/omalloc' make[1]: *** [install] Error 1 make[1]: Leaving directory `/export/home/jaap/Downloads/sage-4.3.2.alpha0/spkg/build/singular-3-1-0-9-20100125/src' make: *** [/export/home/jaap/Downloads/sage-4.3.2.alpha0/local/bin/Singular-3-1-0] Error 2 Unable to build Singular.
Actually this is the same failure I had before.
What is the use of -gp in building singular for sage?
If I remove -gp in Makefile and Makefile.in all, over singular builds on Open Solaris x64 64 bit
Jaap
comment:18 Changed 13 years ago by
Replying to AlexGhitza:
Jaap,
Did the previous Singular spkg build properly on Open Solaris? If so, I wonder if it's just a problem in spkg-install, or a more serious issue with Singular itself.
The -gp flag leads to problems on Open Solaris.
Jaap
comment:19 follow-up: 20 Changed 13 years ago by
man gcc
says:
-pg Generate extra code to write profile information suitable for the analysis program gprof. You must use this option when compiling the source files you want data about, and you must also use it when linking.
so this shouldn't be used by default anyway.
comment:20 Changed 13 years ago by
Replying to malb:
man gcc
says:-pg Generate extra code to write profile information suitable for the analysis program gprof. You must use this option when compiling the source files you want data about, and you must also use it when linking.so this shouldn't be used by default anyway.
+1
Thanks.
Jaap
comment:21 follow-up: 22 Changed 13 years ago by
Martin, do you want to remove this option in the spkg? I'd be happy to do some test builds to make sure that it doesn't break anything on other architectures. But if the fix is this simple to make it build on Open Solaris, we might as well do it now.
comment:22 Changed 13 years ago by
Replying to AlexGhitza:
Martin, do you want to remove this option in the spkg? I'd be happy to do some test builds to make sure that it doesn't break anything on other architectures. But if the fix is this simple to make it build on Open Solaris, we might as well do it now.
I think it need upstream work. In the former release it was in patches. Now it is in source :(
Removing -gp will not break anything in sage. We do no profiling!
Jaap
comment:23 Changed 13 years ago by
Looking at Jaap's error again it comes from omalloc. For omalloc all targets are built which are
- libomalloc (normal)
- libomalloc_ndebug (no debugging symbols)
- libomalloc_p (for profiling)
We shouldn't mess with the -pg
switch but should convince omalloc's autotools not to build the profiling version on top of the other two.
comment:24 Changed 13 years ago by
One more comment. Only developers of a program or other software can put interest in profiling there product.
So once more: let's ask upstream to remove this option.
For now it is an option to introduce some patches again in the spkg-install :(
Shall we set status to needs work?
Jaap
comment:25 Changed 13 years ago by
I would also add I think adding profiling information by default is a bad idea.
- Adding profiling information will slow the code quite considerably.
- The -pg option is not portable, so it would hamper efforts to build with a non-GNU compiler.
- The purposely add an option which is breaking a build on Open Solaris, when an Open Solaris port is in progress, seems a bad idea.
- As Jaap points out, only developers want this information - not end users.
I would be tempted to set it to 'needs work', but feel I can't really do so, since Open Solaris is not supported on Sage. So the fact an option breaks on an unsupported operating system is not a sufficiently good reason for rejecting it. But I would urge the person that created the ticket, or the reviewer to do so. (BTW, the Reviewer field is not filled in).
Dave
comment:26 Changed 13 years ago by
Status: | positive_review → needs_work |
---|
Guys,
no one wants to add -pg to the default options for Singular and no one did! Most Singular libraries are built in several flavours as explained above but not all of them are used of course. It's the build of an unused library that is going wrong.
There is a discussion on this issue on [libsingular-devel]:
http://groups.google.com/group/libsingular-devel/browse_thread/thread/43d157fe730e8798
where Hans Schönemann suggested to patch omalloc. It is still puzzling why the build breaks now since IIRC nothing changed in that area.
comment:27 follow-up: 28 Changed 13 years ago by
Is anybody working on patching the Makefiles? It might take a few days before I'll get around to do it.
comment:28 Changed 13 years ago by
Replying to malb:
Is anybody working on patching the Makefiles? It might take a few days before I'll get around to do it.
Are you getting around to do it?
Jaap
comment:30 Changed 13 years ago by
I thought about this patch-all-Makefiles approach some more. I came to the conclusion that it would be a bad idea. Jaap's GCC or the Singular's configuration script is broken. GCC accepts -pg
and that GCC what is called. That it fails hints at either a bug in the OpenSolaris? GCC or at a problem choosing the right assembler or linker.
Patching all Makefiles would have serious repercussions: this patch would never be accepted upstream and thus we would have to go through a very tedious and error-prone process of patching every Makefile with every SPKG update. Since I am right now the only person who does these updates at the moment, I think I refuse to increase my workload like this based on one faulty setup.
Note that I am happy to investigate and push upstream any fix which triggers this error in the configuration script. I want Sage to build and work on OpenSolaris? and Solaris. It's just this approach which I deem incorrect.
comment:31 Changed 13 years ago by
If there are a lot of Makefiles, then patching would be tedious and require more work with upgrades. However, it should not be too hard to create a small script which patches the Makefiles and call that script from spkg-install.
I've not checked this on the Singular sources, but this sed script, which I called 'nopg' could be used to replace ' -pg ' with nothing
#!/bin/sh sed 's/ -pg //g' $1 > /tmp/Makefile-without-pg-option.$$ mv /tmp/Makefile-without-pg-option.$$ $1
then in spkg-install have:
find src -name Makefile -exec /path/to/nopg {} \;
I tried it on a gcc source distribution, which has loads of 'configure' scripts, replacing 'echo' with 'zzzzzzzzzzzzzzzzz' and sure enough, all the configure script have strings of zzzzz's in them.
It would be necessary to check the files have the required changes, and no other changes, but that should be a simple solution.
Dave
Dave
comment:32 Changed 13 years ago by
PS, the substitution would be from ' -pg ' to nothing at all, so it would not break if files in the makefile have -pg in their names.
comment:33 Changed 13 years ago by
That indeed might be a viable solution. However, we should still investigate what exactly breaks Jaap's setup instead of working around the issue which we don't understand.
comment:34 Changed 13 years ago by
A new SPKG is available at #8228, you will need the attached patch for this ticket to make it work.
comment:35 Changed 13 years ago by
I vote to review 3-1-0-9 without considering Jaap's bug and then opening a new ticket where Jaap gives more details so that we can address the bug. Right now, Sage crashes in a fairly basic situation and we thus should upgrade.
comment:36 follow-up: 37 Changed 13 years ago by
Martin, this is probably not the question you want to hear, but: would it be worth it to go straight to 3-1-1, since it was just released?
comment:37 Changed 13 years ago by
Replying to AlexGhitza:
Martin, this is probably not the question you want to hear, but: would it be worth it to go straight to 3-1-1, since it was just released?
I'll make you a deal: if you review it I'll make a Singular-3-1-1 SPKG :)
Changed 13 years ago by
Attachment: | singular-3-1-1-0.patch added |
---|
comment:39 Changed 13 years ago by
A new SPKG is available at
http://sage.math.washington.edu/home/malb/spkgs/singular-3-1-1-0-20100305.spkg
comment:40 Changed 13 years ago by
There are doctest failures in the scheme subdirectory. I reported one bug upstream already.
comment:41 Changed 13 years ago by
Doctest failures:
The following tests failed: sage -t devel/sage/sage/schemes/plane_curves/projective_curve.py # 14 doctests failed sage -t devel/sage/sage/schemes/plane_curves/curve.py # 6 doctests failed sage -t devel/sage/sage/schemes/plane_curves/constructor.py # 6 doctests failed
comment:42 Changed 13 years ago by
After fixing a typo in a Singular routine
http://groups.google.com/group/libsingular-devel/browse_frm/thread/de4c7123c71f2150
I get a bit further, but I need some help now:
I assume '32' is wrong in the failure below?
sage -t "devel/sage/sage/schemes/plane_curves/constructor.py" ********************************************************************** File "/mnt/usb1/scratch/malb/sage-4.3.3/devel/sage/sage/schemes/plane_curves/constructor.py", line 72: sage: C.genus() Expected: 0 Got: 32
Am I correct that the following is also wrong?
sage -t "devel/sage/sage/schemes/plane_curves/curve.py" ? ideal defines not a curve ? leaving normal.lib::genus ********************************************************************** File "/mnt/usb1/scratch/malb/sage-4.3.3/devel/sage/sage/schemes/plane_curves/curve.py", line 79: sage: C.geometric_genus()
comment:43 Changed 13 years ago by
The different behaviour in pure Singular:
> ring r = 7,(x,y),dp; > LIB("normal.lib"); > ideal i = x^10 + x^3 + y^2; > genus(i); 32
comment:44 Changed 13 years ago by
Martin, I just tried the new spkg but I'm running into trouble very quickly: after doing sage -f ...
, I'm getting errors when doing sage -b
This is under sage-4.3.4.alpha0, and I got it now on two machines (including sage.math)
comment:46 Changed 13 years ago by
Hmmm that's pretty embarrassing.
So it builds fine now. Note that applying the patch to either 4.3.3 or 4.3.4.alpha0 results in one rejection. It is actually harmless, but it might be good to rebase anyway.
I'm pretty sure that the (geometric) genus should be 0 in the doctest that fails claiming it's 32. (In particular, both Macaulay2 and Magma say 0.)
I guess this should be reported upstream, since they know better what they've changed since 3-1-0-9 in normal.lib.
comment:47 follow-up: 48 Changed 13 years ago by
Report Upstream: | N/A → Fixed upstream, in a later stable release. |
---|
The bug I isolated above will be fixed in the next release:
Hi Martin, Thank you for reporting this bug. It will be corrected in the next release. Gert-Martin 2010/3/8 Martin Albrecht <malb@informatik.uni-bremen.de>: > Hi, > > the following output is wrong in 3-1-1-0 and was correct in the previous > version: > >> ring r = 7,(x,y),dp; >> LIB("normal.lib"); >> ideal i = x^10 + x^3 + y^2; >> genus(i); > 32 //should be 0
We should try to make sure that we report all bugs (I think there is another one were curves are considered not to be curves) before they cut the new release.
comment:48 Changed 13 years ago by
We should try to make sure that we report all bugs (I think there is another one were curves are considered not to be curves) before they cut the new release.
Yep. If I have some time (cross fingers) I'll try to test 3-1-1-0 with a few more examples and see if I catch anything else. Any idea what the timeline is for the new release?
comment:49 Changed 13 years ago by
No idea, but I just reported another bug upstream:
> ring r = 5,(x,y,z),dp; > ideal i = -x^3 + y^2*z - 2*x*z^2 + y*z^2; > LIB("normal.lib"); > genus(i); ? ideal defines not a curve ? leaving normal.lib::genus
Another thing we might want to tackle is that it seems that our libsingular interface isn't very good at recovering from errors. I'm not sure how to address this; I guess I'll ask upstream some time.
comment:50 Changed 13 years ago by
Reply from upstream:
This is not a bug, Singular computes only the genus of a curve, as is said in the manual: genus(i) or genus(i,1); I a 1-dimensional ideal In the example the ideal defines a surface. Gert-Martin 2010/3/9 <owner-singular-team@mathematik.uni-kl.de>: > #212: wrong computation of genus / error when calling genus > --------------------------------------------------------------+------------- > Reporter: Martin Albrecht <malb@… | Owner: somebody > Type: defect | Status: new > Priority: minor | Milestone: Releases 3-1-1 and higher > Component: singular-libs | Version: 3-1-1 > Keywords: genus | > --------------------------------------------------------------+------------- > > Comment(by seelisch): > > Here's another bug related to genus: > > > ring r = 5,(x,y,z),dp; > > > ideal i = -x3 + y2*z - 2*x*z2 + y*z2; > > > LIB("normal.lib"); > > > genus(i); > ? ideal defines not a curve > ? leaving normal.lib::genus
Alex, can you look into this?
comment:51 Changed 13 years ago by
Dear Martin, I realize that the example was a homogeneous polynomial in 3 variables. So, this can be interpreted as an affine surface but also as a homogeneous curve. Probably the second was meant. In this case it makes sense to compute the genus. We shall take care about this in the next release. Best, Gert-Martin
comment:52 Changed 13 years ago by
There's a new Singular version at
http://www.mathematik.uni-kl.de/ftp/pub/Math/Singular/SOURCES/3-1-1/
which might or might not fix the bug
comment:53 follow-ups: 54 56 Changed 12 years ago by
I've updated the Singular SPKG to 3-1-1-3 which fixes the genus issue. However, this release segfaults a lot! I'm in the process of resolving this (cf. [libsingular-devel])
http://sage.math.washington.edu/home/malb/spkgs/singular-3-1-1-3.p0.spkg
comment:54 Changed 12 years ago by
Cc: | François Bissey added |
---|---|
Description: | modified (diff) |
Replying to malb:
I've updated the Singular SPKG to 3-1-1-3 which fixes the genus issue. However, this release segfaults a lot! I'm in the process of resolving this (cf. [libsingular-devel])
http://sage.math.washington.edu/home/malb/spkgs/singular-3-1-1-3.p0.spkg
Hi Martin Albrecht , A few comments.
- Is it possible you could request the upstream developers distribute the source code so the files generated by tools like autoconf, flex, actually have later modification times of the generated file newer than the file they are generated from?
Currently libparse.cc, which is the output from 'flex' has exactly the same modification time (to the ns) as the file it was supposedly generated from (libparse.l). I doubt 'flex' actually run in < 1 ns!
-rw-r----- 1 drkirkby staff 109422 2010-02-03 13:58:03.000000000 +0000 src/Singular/libparse.cc -rw-r----- 1 drkirkby staff 30875 2010-02-03 13:58:03.000000000 +0000 src/Singular/libparse.l
Likewise for these files. Clearly autoconf did not run in under a ns.
-rwxr-x--- 1 drkirkby staff 85614 2010-03-03 11:36:23.000000000 +0000 src/factory/configure -rw-r----- 1 drkirkby staff 13992 2010-03-03 11:36:23.000000000 +0000 src/factory/configure.in
This will save us having to 'touch' the files.
- There's a problem with the
build()
part of spkg-install.
The script author probably assumes that if any one of the items in build()
fail, then build()
will fail and so the error will be reported. However, that is not the case. As long as the last item in build()
, which is (install_docs
), does not fail, the return code from build()
will be 0. So if for example I modify the build()
section to:
build() { clean_headers # This SHOULD break the build rm /non-existant-file patch distclean config build_singular build_libsingular build_factory build_libfac create_singular_script install_docs } build if [ $? -ne 0 ]; then echo "Error somewhere building Singular." exit 1 fi
then when I build Singular, I see:
Building a 64-bit version of Singular rm: /non-existant-file: No such file or directory make: *** No rule to make target `distclean'. Stop. creating cache ./config.cache checking uname for singular... ix86-SunOS checking for gcc... gcc -m64 ... Successfully installed singular-3-1-1-3.p1
In other words, the error of trying to delete /non-existant-file does not cause the build to fail, as it should do. That means if the library fails to build, but the docuemtation installs ok, then the script will not show the error. Each part should be checked individually.
I've not checked this, but believe the following would work:
build() { for i in clean_headers patch distclean config build_singular build_libsingular build_factory build_libfac create_singular_script install_docs ; do $i if [ $? -ne 0 ]; then echo "Error building Singular when running '$i'." exit 1 fi done }
- There are in fact a lot of parts in spkg-install which are not checked for errors.
- François Bissey has said the previous version of Singular built some unnecessary targets, which adds to the build time of Singular. Given this is the longest package to build (taking almost 6 minutes on my PC), it would be worth reducing them if you have not already done so. I've cc'ed François on the ticket, as he knows a lot more about this package than me.
- It does built on OpenSolaris, though I've not checked it on anything else. I'll wait until you have sorted out the segfaults first!
PS, The first 3-1-1-3 to go into Sage should be called singular-3-1-1-3.spkg and not have a '.p0' in the name.
Dave
comment:55 Changed 12 years ago by
Thanks for adding me Dave I will update my patch to simplify slightly the build to this version of singular. I know from experience in sage-on-gentoo that sage will build and the tests will be ok if you remove the build_factory and build_libfac targets. As a bonus the creation of the link LIB to lib is 1) bogus 2) useless. In a build of singular LIB usually contains the the *.lib files. Those files are installed in share/singular thanks to some of the instructions in the build. Before being removed and recreated the link actually points to share/singular. Removing the link didn't harm any tests.
The only issue with removing these 2 targets may be the optional Macaulay2 spkg. We have some discussions atm in gentoo about singular - the components it provides - and Macaulay2. So while sage builds ok, Macaulay2 should be checked.
comment:56 Changed 12 years ago by
Replying to malb:
I've updated the Singular SPKG to 3-1-1-3 which fixes the genus issue. However, this release segfaults a lot!
Hi Martin,
do you see this warning message about warning: returning reference to temporary
? I got that warnings when building on OpenSolaris - see below.
g++ -m64 -O2 -g -m64 -fPIC -pipe -fno-implicit-templates -I. -I../kernel -I/export/home/drkirkby/sage-4.5.alpha4/local/include -I/export/home/drkirkby/sage-4.5.alpha4/local/include -I/export/home/drkirkby/sage-4.5.alpha4/local/include -O2 -g -m64 -DNDEBUG -DOM_NDEBUG -Dix86_SunOS -DHAVE_CONFIG_H -c iparith.cc In file included from ../kernel/ring.h:13, from subexpr.h:16, from ipid.h:13, from iparith.cc:20: ../kernel/polys-impl.h:177:1: warning: "pPolyAssumeReturn" redefined ../kernel/polys-impl.h:176:1: warning: this is the location of the previous definition In file included from ../kernel/walkMain.h:5, from ../kernel/walkProc.h:3, from iparith.cc:56: ../kernel/int64vec.h: In member function 'const int& int64vec::operator[](int) const': ../kernel/int64vec.h:51: warning: returning reference to temporary
That sounds the sort of error which could result in segfaults.
I'm attaching a patch which gives much improved error checking.
Dave
Changed 12 years ago by
Attachment: | 8059-improved-error-checking.patch added |
---|
Based on Martin's singular-3-1-1-3 package, but with improved error checking.
comment:57 Changed 12 years ago by
I just realised I copied a bit too little of that warning message. The message is about returning reference to temporary memory:
../kernel/int64vec.h:51: warning: returning reference to temporary memory
That looks like it could result in segfaults to me.
Dave
Changed 12 years ago by
Attachment: | simplify-singular.patch added |
---|
Simplify the building of singular
comment:58 follow-up: 60 Changed 12 years ago by
The only issue with removing these 2 targets may be the optional Macaulay2 spkg. We > have some discussions atm in gentoo about singular - the components it provides - and Macaulay2. So while sage builds ok, Macaulay2 should be checked.
I'm pretty sure Macaulay2 will fail if we drop libfac and libcf. Thus I'm not applying the second patch for now.
comment:59 Changed 12 years ago by
I've applied David's patch to
http://sage.math.washington.edu/home/malb/spkgs/singular-3-1-1-3.spkg
and also changed the name in accordance with his comments. I've also forwarded David's comments to the Singular team at:
http://groups.google.com/group/libsingular-devel/browse_thread/thread/e67e6b2afe1d8314
comment:60 Changed 12 years ago by
Replying to malb:
The only issue with removing these 2 targets may be the optional Macaulay2 spkg. We > have some discussions atm in gentoo about singular - the components it provides - and Macaulay2. So while sage builds ok, Macaulay2 should be checked.
I'm pretty sure Macaulay2 will fail if we drop libfac and libcf. Thus I'm not applying the second patch for now.
It would be good if this Macaulay2 could be made to rebuild Singular in this case. Perhaps have an environment variable that determines if those two targets are built or not.
build() { patch distclean config build_singular build_libsingular if [ "x$SAGE_USING_Macaulay2" = xyes ] ; then build_factory build_libfac fi create_singular_script install_docs }
Then ensure Macaulay2
- exports SAGE_USING_Macaulay2 = "yes"
- Rebuilds Singular. This time build_factory & build_libfac would built, due to the environment variable being set to "yes"
I guess this depends on the popularity of Macaulay2. If 20% of people install it, then we might as well build all targets. If however only 1% of people install Macaulay2, then I doubt it warrants slowing the build process for everyone.
Currently I believe Singular is the slowest package to build in Sage (at least on my OpenSolaris machine), though neither R or Maxima are building, so I can't say for sure. But Singular definately takes a long time to build, so if that could be improved, it would be nice. Of course, it depends on how long those targets take to build. If the savings from not building them are only small, we might as well build them.
Another option might be to let an environment variable be used to stop those two targets building, but by default build them all.
Dave
comment:61 Changed 12 years ago by
To build both singular and Macaulay2 our current approach is too try to build omalloc factory and libfac first, and then move on to the rest of singular/Macaulay2. That approach makes sense in the context of Gentoo. If it works I may try to adapt the singular spkg build process to match it.
It is not a done deal that this approach will work however.
Francois
comment:62 follow-up: 63 Changed 12 years ago by
Report Upstream: | Fixed upstream, in a later stable release. → Reported upstream. Little or no feedback. |
---|
If you don't mind I'd suggest to open a new ticket to address the libfac issue (by including libfac in the Macaulay SPKG for example). Right now, we have crashes in Sage because of bugs in older versions of Singular and I'd like to focus on fixing those for now. With the updated patch I'll attach in a minute, I'm down to two doctest failures with the most recent Singular version:
sage -t devel/sage/sage/rings/polynomial/toy_d_basis.py # 3 doctests failed sage -t devel/sage/sage/rings/polynomial/multi_polynomial_element.py # 4 doctests failed
comment:63 Changed 12 years ago by
Replying to malb:
If you don't mind I'd suggest to open a new ticket to address the libfac issue (by including libfac in the Macaulay SPKG for example).
No problem. That makes sence.
Dave
comment:64 Changed 12 years ago by
I've updated the singular-3-1-1-3.spkg according to Hans' comments on [libsingular-devel] and reverted some overly aggressive changes to singuler-3-1-1-3.patch. With this I'm down to one problem:
sage -t "devel/sage/sage/rings/polynomial/toy_d_basis.py" ********************************************************************** File "/mnt/usb1/scratch/malb/sage-4.4/devel/sage/sage/rings/polynomial/toy_d_basis.py", line 38: sage: gb = d_basis(I); gb Expected: [x - 2020, y - 11313, 22627] Got: [1] ********************************************************************** File "/mnt/usb1/scratch/malb/sage-4.4/devel/sage/sage/rings/polynomial/toy_d_basis.py", line 41: sage: gb[-1].factor() Expected: 11^3 * 17 Got: 1 ********************************************************************** File "/mnt/usb1/scratch/malb/sage-4.4/devel/sage/sage/rings/polynomial/toy_d_basis.py", line 208: sage: gb = d_basis(I); gb Expected: [x - 2020, y - 11313, 22627] Got: [1]
Changed 12 years ago by
Attachment: | singular-3-1-1-4.patch added |
---|
comment:65 Changed 12 years ago by
Status: | needs_work → needs_review |
---|
The Singular team cut us a new release which
- passes all doctests on sage.math
- supports parallel make
The SPKG is at
http://sage.math.washington.edu/home/malb/spkgs/singular-3-1-1-4.spkg
comment:66 Changed 12 years ago by
On sage.math:
make -j 20
real 3m55.543 user 8m10.720 sys 0m49.920s
make
real 8m12.656s user 7m39.400s sys 0m43.580s
comment:67 Changed 12 years ago by
Reviewers: | → David Kirkby |
---|---|
Status: | needs_review → needs_work |
On a Sun Ultra 27 (3.33 GHz, quad core, 8 threads), running OpenSolaris:
drkirkby@hawk:~/sage-4.5.rc0$ echo $MAKE make -j12
the build fails with:
g++ -m64 -O2 -g -m64 -fPIC -fno-implicit-templates -I./factor -I./charset -I. -I./factor -I/export/home/drkirkby/sage-4.5.rc0/local/include -I/export/home/drkirkby/sage-4.5.rc0/local/include -I/export/home/drkirkby/sage-4.5.rc0/local/include -O2 -g -m64 -DHAVE_CONFIG_H -c charset/csutil.cc -o OPTOBJ/csutil.o g++ -m64 -O2 -g -m64 -fPIC -fno-implicit-templates -I./factor -I./charset -I. -I./factor -I/export/home/drkirkby/sage-4.5.rc0/local/include -I/export/home/drkirkby/sage-4.5.rc0/local/include -I/export/home/drkirkby/sage-4.5.rc0/local/include -O2 -g -m64 -DHAVE_CONFIG_H -c charset/charset.cc -o OPTOBJ/charset.o Assembler messages: Fatal error: can't create OPTOBJ/version.o: No such file or directory g++ -m64 -O2 -g -m64 -fPIC -fno-implicit-templates -I./factor -I./charset -I. -I./factor -I/export/home/drkirkby/sage-4.5.rc0/local/include -I/export/home/drkirkby/sage-4.5.rc0/local/include -I/export/home/drkirkby/sage-4.5.rc0/local/include -O2 -g -m64 -DHAVE_CONFIG_H -c charset/reorder.cc -o OPTOBJ/reorder.o g++ -m64 -O2 -g -m64 -fPIC -fno-implicit-templates -I./factor -I./charset -I. -I./factor -I/export/home/drkirkby/sage-4.5.rc0/local/include -I/export/home/drkirkby/sage-4.5.rc0/local/include -I/export/home/drkirkby/sage-4.5.rc0/local/include -O2 -g -m64 -DHAVE_CONFIG_H -c charset/alg_factor.cc -o OPTOBJ/alg_factor.o make[2]: *** [OPTOBJ/version.o] Error 1 make[2]: *** Waiting for unfinished jobs.... mkdir OPTOBJ make[2]: Leaving directory `/export/home/drkirkby/sage-4.5.rc0/spkg/build/singular-3-1-1-4/src/libfac' make[1]: *** [install] Error 1 make[1]: Leaving directory `/export/home/drkirkby/sage-4.5.rc0/spkg/build/singular-3-1-1-4/src' make: *** [/export/home/drkirkby/sage-4.5.rc0/local/bin/Singular-3-1-1] Error 2 Unable to build Singular. real 0m18.232s user 1m18.531s sys 0m8.304s sage: An error occurred while installing singular-3-1-1-4
The previous version did build on this machine. It looks to me like the parallel building is not working correctly, as it is trying to use Have you checked on 't2.amth' and/or 'bsd.math'?
I simply don't have time to check everyone's packages on Solaris. I alway try to check that mind at least build on Solaris, Linux and OS X.
Dave
comment:68 follow-up: 70 Changed 12 years ago by
I've replaced the SPKG with one where the dependencies are fixed. As soon as I have a working Sage installation on t2 I'll try to build it there in parallel.
PS: Please allow me a bit more than 20 minutes to test a package on Solaris next time before you complain that I didn't test it yet.
comment:69 follow-up: 71 Changed 12 years ago by
on t2.math.washington.edu with MAKE="make -j32"
real 36m43.246s user 61m52.416s sys 4m55.036s Successfully installed singular-3-1-1-4 Now cleaning up tmp files. rm: Cannot remove any directory in the path of the current working directory /home/malb/t2/sage-4.3.0.1-Solaris-10-SPARC-sun4u-or-sun4v/spkg/build/singular-3-1-1-4 Making Sage/Python scripts relocatable... Making script relocatable Finished installing singular-3-1-1-4.spkg
David, is that faster or slower than without the parallel build option?
comment:70 Changed 12 years ago by
Replying to malb:
I've replaced the SPKG with one where the dependencies are fixed. As soon as I have a working Sage installation on t2 I'll try to build it there in parallel.
Thank you.
PS: Please allow me a bit more than 20 minutes to test a package on Solaris next time before you complain that I didn't test it yet.
Well it was marked for review. I think if you want a reviewer to look at it, you should have checked it on the support platforms - Linux, OS X and Solaris. I know 't2' is not the fastest machine in the world, but with parallel builds, it is pretty decent. Basically the T5240 is designed for a very different task to what we use it for.
Dave
comment:71 Changed 12 years ago by
Replying to malb:
on t2.math.washington.edu with MAKE="make -j32"
real 36m43.246s user 61m52.416s sys 4m55.036s Successfully installed singular-3-1-1-4 Now cleaning up tmp files. rm: Cannot remove any directory in the path of the current working directory /home/malb/t2/sage-4.3.0.1-Solaris-10-SPARC-sun4u-or-sun4v/spkg/build/singular-3-1-1-4 Making Sage/Python scripts relocatable... Making script relocatable Finished installing singular-3-1-1-4.spkgDavid, is that faster or slower than without the parallel build option?
I can't tell you that for sure - you would need to test it. However, the fact it has used 67 minutes of CPU time, but took 37 minutes to build, suggests the parallel build is working. I doubt any overhead could explain that difference.
That should be quite useful, as Singular is probably in the top 3 of packages taking the longest to build. In fact, it might have the #1 spot. So it's worth making Singular work in parallel, whereas for some packages, it's hardly worth the bother.
Dave
comment:72 Changed 12 years ago by
on bsd.math.washington.edu with MAKE=make -j8
real 6m12.144s user 8m16.609s sys 1m5.636s
comment:73 Changed 12 years ago by
REFEREE REPORT:
- builds and passes all tests with sage-4.5 on sage.math
- everything in spkg-install looks fine to me.
There are a bunch of .cvsignore files all over:
wstein@sage:~/build/sage-4.5.alphastein1/singular-3-1-1-4/src$ hg status|grep "\!"|grep cvsignore ! IntegerProgramming/.cvsignore ! Singular/.cvsignore ! doc/.cvsignore ! factory/.cvsignore ! factory/ftest/.cvsignore ! kernel/.cvsignore ! libfac/.cvsignore ! omalloc/.cvsignore
I will give this a positive review if:
- the cvsignore's are removed
- build testing passes on everything
comment:74 Changed 12 years ago by
Hey, I'm wrong -- you got *rid* of the .cvsignores!
This gets a positive so long as it builds on stuff.
comment:75 Changed 12 years ago by
Hey, I'm wrong -- you got *rid* of the .cvsignores!
This gets a positive so long as it builds on stuff.
comment:80 Changed 12 years ago by
I didn't manage to build R on Itaninum, thus I do get some doctest failures. The genus failure is also unrelated to this ticket, thus I claim it tests successfully on Itanium Linux
sage -t "devel/sage/sage/interfaces/r.py" sage -t "devel/sage/sage/interfaces/expect.py" sage -t "devel/sage/sage/stats/test.py" sage -t "devel/sage/sage/stats/r.py" sage -t "devel/sage/sage/graphs/genus.pyx"
comment:81 Changed 12 years ago by
Are there any Singular self-tests? There is no spkg-check
file, so these are not run if SAGE_CHECK is set to yes.
comment:82 Changed 12 years ago by
Status: | needs_work → needs_review |
---|
comment:83 Changed 12 years ago by
Status: | needs_review → positive_review |
---|
comment:84 Changed 12 years ago by
Status: | positive_review → needs_info |
---|
This currently has positive review, and I'm listed as the reviewer, though I have not given it positive review.
From what I see above, the only evidence of this working on Solaris is that the package builds. It's not clear Sage can even build on Solaris with this.
Dave
comment:86 follow-up: 87 Changed 12 years ago by
Should this be called "singular....p0.spkg", since there is some patching involved? Rather than 3-1-1-4, should the version number be 3.1.1.4? That seems to be the current style for singular in sage, but I've seen it both ways.
I'm going to try building this on t2 later tonight. I'll restore the positive review if it works.
comment:87 Changed 12 years ago by
Replying to jhpalmieri:
Should this be called "singular....p0.spkg", since there is some patching involved? Rather than 3-1-1-4, should the version number be 3.1.1.4? That seems to be the current style for singular in sage, but I've seen it both ways.
I do not believe it should have a .p0, since its never actually gone into Sage at this point. So if further changes are made, it can still remain 3.1.1.4. That's my understanding of the situation.
Dave
comment:88 Changed 12 years ago by
Status: | needs_info → needs_work |
---|
At least with Sage 4.5.2.alpha0, while this builds on several platforms (I haven't tried on Solaris yet), the Sage library fails to build. I get an error like this:
sage/libs/singular/function.cpp: In function ‘void initfunction()’: sage/libs/singular/function.cpp:14355: error: ‘__pyx_f_4sage_5rings_10polynomial_34multi_polynomial_ideal_libsingular_sage_ideal_to_singular_ideal’ was not declared in this scope make[1]: *** [installed/sage-4.5.2.alpha0] Error 1
(This is with sage-4.5.2.alpha0 plus the singular spkg here, attempting to build from scratch.)
comment:89 Changed 12 years ago by
More specifically, I have failures on
- sage.math
- skynet machine taurus (x86_64-Linux-nehalem-fc) -- built from scratch twice, failed the same way both times
- skynet machine eno (x86_64-Linux-core2-fc) -- on an existing sage 4.5.2.alpha0 install, tried just "sage -i ..." then "sage -ba" fails
- an OS X 10.6 box -- tried "sage -i ..." then "sage -ba" fails
Maybe it's not compatible with #1396?
comment:90 follow-up: 93 Changed 12 years ago by
Hi jhpalmieri, can you give me instructions how to reproduce this problem, i.e. what patches you've merged etc.?
comment:91 Changed 12 years ago by
comment:92 Changed 12 years ago by
@jhpalmieri The spkg does need singular-3-1-1-4.patch to be applied to the Sage library.
comment:93 Changed 12 years ago by
Replying to malb:
Hi jhpalmieri, can you give me instructions how to reproduce this problem, i.e. what patches you've merged etc.?
I took a tarball for Sage 4.5.2.alpha0, dropped in the new singular spkg (singular-3-1-1-4.spkg), and ran "make". I'll try again now with an already built version of Sage, running "sage -i singular-3-1-1-4.spkg" and then installing the 3-1-1-4 patch -- I hadn't done that before.
comment:94 Changed 12 years ago by
Oh, the patch doesn't apply cleanly to Sage 4.5.2.alpha0:
applying /home/palmieri/singular-3-1-1-4.patch patching file sage/libs/singular/option.pyx Hunk #1 succeeded at 108 with fuzz 1 (offset 14 lines). Hunk #2 FAILED at 313 Hunk #3 FAILED at 349 2 out of 3 hunks FAILED -- saving rejects to file sage/libs/singular/option.pyx.rej patching file sage/libs/singular/singular-cdefs.pxi Hunk #9 succeeded at 790 with fuzz 1 (offset -10 lines). patching file sage/libs/singular/singular.pyx Hunk #1 succeeded at 30 with fuzz 1 (offset 0 lines).
Sage does start after this, though. There are a few doctest failures, but they look like random unrelated ones.
Note that #1396 may be backed out of Sage 4.5.2, so you might want to coordinate with that patch rather than just rebasing against some version of 4.5.2. Also, please test on t2.math or another solaris box: the patch at #1396 caused a segfault on t2.math but not on any other platform...
comment:95 follow-up: 96 Changed 12 years ago by
As a note, #1396 was backed out, despite the fact it is still shows as fixed. I've suggested to the release manager that he changes it to "need work" which would avoid the confusion.
Is there any progress on this ticket? I created #9397 with the aim of making a small change to the current version of Singular in Sage, so it would build 64-bit on Solaris. I then set it from 'needs review' to 'needs info' thinking there was no point in getting that reviewed if this ticket gets merged. (It had at the time got positive review).
But this has been open 6 months now, and no comments added for 7 days. I'm tempted to try to get #9397 reviewed.
Dave
comment:96 Changed 12 years ago by
Replying to drkirkby:
But this has been open 6 months now, and no comments added for 7 days. I'm tempted to try to get #9397 reviewed.
Sorry, I had been working on different things.
On t2, I have applied the patch to sage 4.5.1 and installed the singular-3-1-1-4.spkg. Then, I did make ptestlong
. Result:
The following tests failed: sage -t -long devel/sage/sage/schemes/elliptic_curves/ell_rational_fiel d.py # File not found sage -t -long devel/sage/sage/schemes/elliptic_curves/ell_curve_isogeny .py # File not found sage -t -long devel/sage/sage/modular/ssmod/ssmod.py # File not found
This seems a bit strange to me.
First, why can the files not be found?
Second, why is it not mentioned in the summary that some tests in fact timed out? Namely:
sage -t -long devel/sage/sage/schemes/elliptic_curves/ell_rational_field.py *** *** Error: TIMED OUT! PROCESS KILLED! *** *** *** *** Error: TIMED OUT! *** *** [1801.8 s] sage -t -long devel/sage/sage/schemes/elliptic_curves/ell_curve_isogeny.py *** *** Error: TIMED OUT! PROCESS KILLED! *** *** *** *** Error: TIMED OUT! *** *** sage -t -long devel/sage/sage/modular/ssmod/ssmod.py *** *** Error: TIMED OUT! PROCESS KILLED! *** *** *** *** Error: TIMED OUT! *** *** [1800.9 s]
Since I am not experienced with t2, I don't know if this doctest result is good or bad.
comment:97 follow-up: 98 Changed 12 years ago by
Hi Simon,
These messages ("File not found") are more or less typical when the doctests time out. I think people are working on improving that part of doctest reporting. Since t2 can be pretty slow, some timeouts are not unusual. I wouldn't worry about the ones you saw. If you really want to try again, try export SAGE_TIMEOUT_LONG=6000
to change the time out for long tests from 1800 seconds to 6000 seconds; then you shouldn't see these. (6000 seconds is a lot more than you should need, even on t2.)
comment:98 follow-up: 100 Changed 12 years ago by
Replying to jhpalmieri:
These messages ("File not found") are more or less typical when the doctests time out. ... If you really want to try again, try
export SAGE_TIMEOUT_LONG=6000
OK, doing it now.
Thanks, Simon
comment:99 Changed 12 years ago by
I'v'e just edited
/usr/local/gcc-4.4.1-sun-linker/gcc441sun
the file I recommend people source, and added to it:
SAGE_TIMEOUT=1000 export SAGE_TIMEOUT SAGE_TIMEOUT_LONG=6000 export SAGE_TIMEOUT_LONG
I've found on my own 900 MHz SPARC that the default SAGE_TIMEOUT_LONG (1800 s) is just about long enough, but the SAGE_TIMEOUT (360 s) is too far too short. I'm not even sure if 1000 seconds for SAGE_TIMEOUT is enough on t2, but it probably is.
Obviously one can unset those variables if one wants, but it would seem sensible to have the defaults so tests should pass.
Dave
comment:100 Changed 12 years ago by
Reviewers: | David Kirkby → David Kirkby, Simon King, William Stein, John Palmieri |
---|---|
Work issues: | → rebase patch |
Replying to SimonKing:
Replying to jhpalmieri:
These messages ("File not found") are more or less typical when the doctests time out. ... If you really want to try again, try
export SAGE_TIMEOUT_LONG=6000
OK, doing it now.
Again, the setting is: Sage 4.5.1 plus singular-3-1-1-4.patch plus singular-3-1-1-4.spkg. sage -ptestlong
on t2 with export SAGE_TIMEOUT_LONG=6000
results in exactly one doctest failure:
sage -t -long devel/sage/sage/parallel/decorate.py ********************************************************************** File "/scratch/sking/sage-4.5.1-Solaris_10_SPARC-sun4u-SunOS/devel/sage-main/sag e/parallel/decorate.py", line 152: sage: v = list(f([1,2,4])); v.sort(); v Exception raised: Traceback (most recent call last): File "/scratch/sking/sage-4.5.1-Solaris_10_SPARC-sun4u-SunOS/local/bin/nca doctest.py", line 1231, in run_one_test self.run_one_example(test, example, filename, compileflags) File "/scratch/sking/sage-4.5.1-Solaris_10_SPARC-sun4u-SunOS/local/bin/sag edoctest.py", line 38, in run_one_example OrigDocTestRunner.run_one_example(self, test, example, filename, compile flags) File "/scratch/sking/sage-4.5.1-Solaris_10_SPARC-sun4u-SunOS/local/bin/nca doctest.py", line 1172, in run_one_example compileflags, 1) in test.globs File "<doctest __main__.example_4[9]>", line 1, in <module> v = list(f([Integer(1),Integer(2),Integer(4)])); v.sort(); v###line 152: sage: v = list(f([1,2,4])); v.sort(); v File "/scratch/sking/sage-4.5.1-Solaris_10_SPARC-sun4u-SunOS/local/lib/pyt hon/site-packages/sage/parallel/multiprocessing_sage.py", line 64, in parallel_i ter p = Pool(processes) File "/scratch/sking/sage-4.5.1-Solaris_10_SPARC-sun4u-SunOS/local/lib/pyt hon2.6/multiprocessing/__init__.py", line 227, in Pool return Pool(processes, initializer, initargs) File "/scratch/sking/sage-4.5.1-Solaris_10_SPARC-sun4u-SunOS/local/lib/pyt hon2.6/multiprocessing/pool.py", line 104, in __init__ w.start() File "/scratch/sking/sage-4.5.1-Solaris_10_SPARC-sun4u-SunOS/local/lib/pyt hon2.6/multiprocessing/process.py", line 104, in start self._popen = Popen(self) File "/scratch/sking/sage-4.5.1-Solaris_10_SPARC-sun4u-SunOS/local/lib/pyt hon2.6/multiprocessing/forking.py", line 94, in __init__ self.pid = os.fork() OSError: [Errno 12] Not enough space ********************************************************************** 1 items had failures: 1 of 12 in __main__.example_4 ***Test Failed*** 1 failures. For whitespace errors, see the file /home/SimonKing/.sage//tmp/.doctest_decorate .py [46.5 s]
So, not enough space. Does that mean the hard disc (or at least /scratch
or my home directory) is too full?
Anyway, I repeated the failing test separately, and then it succeeded:
sage subshell$ sage -t -long devel/sage/sage/parallel/decorate.py sage -t -long "devel/sage/sage/parallel/decorate.py" [48.8 s] ---------------------------------------------------------------------- All tests passed! Total time for all tests: 48.8 seconds
So, can one say that all tests pass on t2?
Looking at the previous posts, it seems that the status is:
- "builds and passes all tests with sage-4.5" on sage.math (William)
- "All tests pass on bsd.math" (Martin)
- "Amazingly, this fully works on Cygwin!!" (William)
- With a little effort (extending timeout and running one test separately) all tests of sage-4.5.1+patch+spkg pass on t2.
- "the patch doesn't apply cleanly to Sage 4.5.2.alpha0" (John).
Do you agree that this is a positive review once the patch is rebased, and that William, John and myself should be added to the list of referees?
Cheers,
Simon
comment:101 follow-up: 102 Changed 12 years ago by
comment:102 Changed 12 years ago by
Replying to jhpalmieri:
Do you agree that this is a positive review once the patch is rebased, and that William, John and myself should be added to the list of referees?
That sounds okay to me. The rebasing should perhaps be coordinated with #9599 (the re-merging of #1396).
I don't think that much co-ordination is needed here. Apparently this ticket is more important than #9599, and #9599 will certainly not happen without the patch from here.
So, I'll simply wait until a rebased version of singular-3-1-1-4.patch is available, and will then base on it a new version of my patch from #1396 for re-merging -- which then needs to be tested for segfault on t2, of course.
comment:103 follow-up: 104 Changed 12 years ago by
With #1396 backed out, this applies, so it doesn't need to be rebased. (One hunk applies "with fuzz".) The commit message doesn't include the trac number, but that's the only problem.
So should it get a positive review now?
comment:104 follow-up: 105 Changed 12 years ago by
Replying to jhpalmieri:
With #1396 backed out, this applies, so it doesn't need to be rebased. (One hunk applies "with fuzz".)
Good!
The commit message doesn't include the trac number, but that's the only problem.
Well, I recently got a "needs work" for the same reason...
So should it get a positive review now?
I wouldn't oppose. Any veto?
comment:105 Changed 12 years ago by
Status: | needs_work → positive_review |
---|
Replying to SimonKing:
Replying to jhpalmieri:
With #1396 backed out, this applies, so it doesn't need to be rebased. (One hunk applies "with fuzz".)
Good!
The commit message doesn't include the trac number, but that's the only problem.
Well, I recently got a "needs work" for the same reason...
So should it get a positive review now?
I wouldn't oppose. Any veto?
Me neither. I'm going to set it to positive review.
Dave
comment:106 follow-ups: 108 109 Changed 12 years ago by
Work issues: | rebase patch |
---|
I'm working on 4.5.3.alpha0, which will contain a mix of spkg updates and repository patches from report {32}. Should I attempt to merge this ticket into 4.5.3 or wait for a dedicated Sage release? Also, can someone indicate exactly which spkg and patch to apply? Thanks!
comment:107 Changed 12 years ago by
Also also: Please ensure that any patches to merge include the ticket number in the first lines of their commit strings.
comment:108 Changed 12 years ago by
Replying to mpatel:
I'm working on 4.5.3.alpha0, which will contain a mix of spkg updates and repository patches from report {32}. Should I attempt to merge this ticket into 4.5.3 or wait for a dedicated Sage release? Also, can someone indicate exactly which spkg and patch to apply? Thanks!
Does this have to be called 4.5.3? I'd feel a lot happier calling it 4.6.0 if there's a major upgrade like Singular. But I think you should try to upgrade Singular.
I don't see the need of a dedicated release - there are lots of updates that have almost no chance of conflicting with this.
I guess I'm one of the people that believes increments in the last digit are just minor changes (bug fixes) and not major new components. It's just the release number I don't like - I think its right to merge this, and some .spkg updates. Pari seems to have stalled again, so I'd go for Singular.
That said, I seem to be in a minority who feel version numbers should reflect the sort of updates that take place. Almost everyone seems happy with random numbers!
Dave
comment:109 follow-up: 110 Changed 12 years ago by
Hi!
Replying to mpatel:
Also, can someone indicate exactly which spkg and patch to apply? Thanks!
I think it is singular-3-1-1-4.patch (but this needs to be rebased) and http://sage.math.washington.edu/home/malb/spkgs/singular-3-1-1-4.spkg
Best regards, Simon
comment:110 Changed 12 years ago by
Replying to SimonKing:
Hi!
Replying to mpatel:
Also, can someone indicate exactly which spkg and patch to apply? Thanks!
I think it is singular-3-1-1-4.patch (but this needs to be rebased) and http://sage.math.washington.edu/home/malb/spkgs/singular-3-1-1-4.spkg
I think that's right, and I don't think it needs to be rebased. At least, I've built on a few machines using this combination, and the patch applied cleanly (albeit with some fuzz), and tests passed.
Changed 12 years ago by
Attachment: | singular-3-1-1-4.2.patch added |
---|
Updated commit string. Use with singular-3-1-1-4.spkg
.
comment:111 Changed 12 years ago by
Thanks. I'll use
- singular-3-1-1-4.2.patch (updated commit string)
- http://sage.math.washington.edu/home/malb/spkgs/singular-3-1-1-4.spkg
comment:112 follow-up: 119 Changed 12 years ago by
Status: | positive_review → needs_work |
---|
There might still be a problem with parallel builds. With 4.5.2 on sage.math, I applied singular-3-1-1-4.2.patch, copied singular-3-1-1-4.spkg to SAGE_ROOT
, and ran
#!/bin/bash set -o pipefail JOBS=20 RUNS=50 for I in `seq $RUNS`; do LOG="singular-3-1-1-4-j$JOBS.log.$I" if [ ! -f "$LOG" ]; then env MAKE="make -j$JOBS" ./sage -f singular-3-1-1-4.spkg 2>&1 | tee "$LOG" CODE=$? echo $0 run $I of $RUNS: code= $CODE fi done
All runs ended with exit code 0 (maybe I didn't retrieve the code correctly?), but
grep "An error occurred" singular-3-1-1-4*log* | sort -n
shows that 21 of the 50 runs failed.
I've put the logs here.
According to
$ grep "No such file or dir" sing*log* | grep -v "cannot remove" | cut -d ':' -f 2- | sort | uniq -c 1 abs_fac.cc:2:21: error: factory.h: No such file or directory 1 bifac.cc:1:21: error: factory.h: No such file or directory 1 cntrlc.o: No such file or directory 11 g++: cntrlc.o: No such file or directory 28 g++: extra.o: No such file or directory 4 g++: feOpt.o: No such file or directory 1 g++: g++: cntrlc.o: No such file or directory 4 g++: g++: extra.o: No such file or directoryextra.o: No such file or directory 2 g++: misc_ip.o: No such file or directory 1 lgs.h:13:21: error: factory.h: No such file or directory
and some digging, the gcc -o gentable
and gcc -o gentable2
commands, at least, sometimes don't have all of their dependencies already built. For example, singular-3-1-1-4-j20.log.14
contains
g++ -O3 -g -fPIC -pipe -I. -I../kernel -I/mnt/usb1/scratch/mpatel/tmp/sage-4.5.2-singular/local/include -I/mnt/usb1/scratch/mpatel/tmp/sage-4.5.2-singular/local/include -I/mnt/usb1/scratch/mpatel/tmp/sage-4.5.2 -singular/local/include -fno-implicit-templates -DNDEBUG -DOM_NDEBUG -Dx86_64_Linux -DHAVE_CONFIG_H -DGENTABLE \ -o gentable1 claptmpl.o iparith.cc tesths.cc mpsr_Tok.cc \ grammar.o scanner.o attrib.o eigenval_ip.o extra.o fehelp.o feOpt.o ipassign.o ipconv.o ipid.o iplib.o ipprint.o ipshell.o lists.o sdb.o fglm.o interpolation.o silink.o subexpr.o janet.o wrapper.o libparse.o sing_win.o gms.o pcv.o maps_ip.o walk.o walk_ip.o cntrlc.o misc_ip.o calcSVD.o Minor.o MinorProcessor.o MinorInterface.o slInit_Dynamic.o -ldl -rdynamic -L../kernel -lkernel -L/mnt/usb1/scratch/mpat el/tmp/sage-4.5.2-singular/local/lib -L/mnt/usb1/scratch/mpatel/tmp/sage-4.5.2-singular/local/lib -lm -lsingfac -lsingcf -lntl -lgmp -lreadline -lncurses -lm -lomalloc ../kernel/mmalloc.o g++ -O3 -g -fPIC -pipe -I. -I../kernel -I/mnt/usb1/scratch/mpatel/tmp/sage-4.5.2-singular/local/include -I/mnt/usb1/scratch/mpatel/tmp/sage-4.5.2-singular/local/include -I/mnt/usb1/scratch/mpatel/tmp/sage-4.5.2 -singular/local/include -fno-implicit-templates -DNDEBUG -DOM_NDEBUG -Dx86_64_Linux -DHAVE_CONFIG_H -DGENTABLE \ -o gentable2 claptmpl.o iparith.cc tesths.cc mpsr_Tok.cc \ grammar.o scanner.o attrib.o eigenval_ip.o extra.o fehelp.o feOpt.o ipassign.o ipconv.o ipid.o iplib.o ipprint.o ipshell.o lists.o sdb.o fglm.o interpolation.o silink.o subexpr.o janet.o wrapper.o libparse.o sing_win.o gms.o pcv.o maps_ip.o walk.o walk_ip.o cntrlc.o misc_ip.o calcSVD.o Minor.o MinorProcessor.o MinorInterface.o slInit_Dynamic.o -ldl -rdynamic -L../kernel -lkernel -L/mnt/usb1/scratch/mpat el/tmp/sage-4.5.2-singular/local/lib -L/mnt/usb1/scratch/mpatel/tmp/sage-4.5.2-singular/local/lib -lm -lsingfac -lsingcf -lntl -lgmp -lreadline -lncurses -lm -lomalloc ../kernel/mmalloc.o g++: extra.o: No such file or directory g++: extra.o: No such file or directory
Please see singular-3-1-1-4-j20.log.47
for the factory.h
errors.
If I didn't make a mistake: What if we return to serial builds for this ticket but open a new one for building Singular in parallel?
comment:113 Changed 12 years ago by
I hope this ticket does finally get merged. It solves all my Solaris issues, and has less patches.
However, I would appreciate if someone could review #9397, which does not update Singular, but has a couple of changes needed to get 64-bit builds of Singular on Solaris.
All the changes are in this ticket, so if this ticket gets merged, then there's no need for #9397. But clearly there is a possibility this ticket will cause problems and not get merged, so it would be nice if #9397 could be merged in that case. But it needs review.
Dave
comment:114 Changed 12 years ago by
I would like to mention at this stage that we have adopted this patch in sage-on-gentoo and use it with singular-3.1.1.4 from the system (we had to fix the path for dlopen in singular.pyx - the only place in the whole sage spkg where there is a dlopen call). We are quite happy with it on x86 and amd64, I should fully test on ppc as well, probably over the next week end (it takes that much time).
Changed 12 years ago by
Attachment: | trac_8059_spkg-restore_serial_build.patch added |
---|
Restore serial builds. SPKG repo patch.
comment:115 Changed 12 years ago by
I've made
http://sage.math.washington.edu/home/mpatel/trac/8059/singular-3-1-1-4.p0.spkg
which restores building the package in serial, for now. The changes are in trac_8059_spkg-restore_serial_build.patch. The p0 package installs consistently for me on sage.math (47 good runs out of 47, so far).
Thoughts?
comment:116 Changed 12 years ago by
I think that changing to serial is fine for now. I just re-ran some of your tests, but with JOBS=4
instead of JOBS=20
. It worked fine. I've seen this before with some other packages, especially on t2: if you build with too many threads, it doesn't work, but it's fine with not as many. (For example, mpir pretty consistently doesn't seem to work for me on t2 with MAKE='make -j12', but MAKE='make -j4' is fine.)
Is this ready for review?
comment:117 Changed 12 years ago by
Status: | needs_work → needs_review |
---|
For what it's worth, with 4.5.3.alpha0 + singular-3-1-1-4.p0.spkg + singular-3-1-1-4.2.patch, the long doctests pass for me on bsd and sage.math.
comment:118 Changed 12 years ago by
Status: | needs_review → positive_review |
---|
Since the only change is to disable parallel building, I think we can restore the positive review.
comment:119 follow-up: 120 Changed 12 years ago by
Cc: | Alexander Dreyer added |
---|
I this this issue occurs only if you have a lot of CPU cores, but slow hard disks. Anyway, the patch at think http://www.singular.uni-kl.de:8002/trac/ticket/250 should cure it.
comment:120 Changed 12 years ago by
Replying to AlexanderDreyer:
I this this issue occurs only if you have a lot of CPU cores, but slow hard disks. Anyway, the patch at think http://www.singular.uni-kl.de:8002/trac/ticket/250 should cure it.
That looks a very trivial change. If I understand correctly, just one line
$(basefactorysrc:.cc=.o): factory.h
is added. But it might be better to put it on another ticket, otherwise this could drag on for ages - the ticket is already 7-months old.
Singular is one of the slowest packages build for me on my Ultra 27, taking about 8 minutes if I recall correctly. But I'm concerned that delaying this much longer will cause more problems than a few minutes of wall time will make.
BTW, I've run with 1000 threads on systems with only 4 cores. It's not optimal, but does allow a reasonable simulation of larger parallel builds to be made.
A big problem in my opinion, is that the server disk.math, which serves all the home directories has been mis-configured to increase the speed of NFS. So failures on home directories might not be code errors, but simply mis-configuration of the server.
Dave
comment:121 Changed 12 years ago by
Yeah, this should be another ticket. BTW, I also hat to a another trivial fix to fix the gentable issue during parallel build.
comment:123 Changed 12 years ago by
Description: | modified (diff) |
---|
comment:124 Changed 12 years ago by
Summary: | update Singular SPKG to newest upstream release → Update Singular spkg to release 3-1-1-4 |
---|
comment:125 Changed 12 years ago by
Merged in: | → sage-4.5.3.alpha0 |
---|---|
Resolution: | → fixed |
Status: | positive_review → closed |
comment:126 Changed 12 years ago by
Merged in: | sage-4.5.3.alpha0 → sage-4.5.3.alpha1 |
---|
comment:127 follow-ups: 128 129 Changed 12 years ago by
Almost all generated files (configure
, grammar.cc
etc.) again carry the same time stamp as their sources... :(
I wonder if we should touch all of them in spkg-install
, not just two.
<flame> I thought the upstream developers had learned at Sage Days 23.5, but perhaps 3.1.1.4 was just prepared too early... </flame>
comment:128 Changed 12 years ago by
Replying to leif:
Almost all generated files (
configure
,grammar.cc
etc.) again carry the same time stamp as their sources... :(
I don't know what method they use to create the tarball, but its hard to see how this can happen. Unless they touch all files before releasing, it's hard to understand how they can created a configure script with the same timestamp as the file it is generated from.
Perhaps they copy them with cp
at one point, and don't preserve modification times (cp -p
). Whatever it is they are doing, it is wrong.
I wonder if we should touch all of them in
spkg-install
, not just two.
Anything that should be touched should be.
<flame> I thought the upstream developers had learned at Sage Days 23.5, but perhaps 3.1.1.4 was just prepared too early... </flame>
No comment.
BTW, is this really 3-1-1-4, or is it a snapshot? If the latter, then it's quite possible someone in the Sage community created the configure script and did not preserve modification times. In which case, the upstream developers are not to blame.
Dave
comment:129 follow-ups: 130 131 Changed 12 years ago by
Almost all generated files (
configure
,grammar.cc
etc.) again carry the same time stamp as their sources... :(
These files can be generated, but there is no need to do so (if you are not a Singular-kernel developer). In fact they are pre-generated in the repo: http://www.singular.uni-kl.de/svn/trunk/ (That's why the time stamps are equal.)
The reason for this is that the generators for these files (e.g. specific version of autotools and lex) are not available on all supported platforms.
Of course, there are attempts to change this, but this cannot be done within 2,5 days. If (and only if) the tools of Sage distribution are capable of generating these files, then the correct solution would be to to remove them complete during patching.
<flame> I thought the upstream developers had learned at Sage Days 23.5, but perhaps 3.1.1.4 was just prepared too early... </flame>
This wasn't at topic at SD23.5.
comment:130 Changed 12 years ago by
Replying to AlexanderDreyer:
Almost all generated files (
configure
,grammar.cc
etc.) again carry the same time stamp as their sources... :(These files can be generated, but there is no need to do so (if you are not a Singular-kernel developer). In fact they are pre-generated in the repo: http://www.singular.uni-kl.de/svn/trunk/ (That's why the time stamps are equal.)
The reason for this is that the generators for these files (e.g. specific version of autotools and lex) are not available on all supported platforms.
Of course, there are attempts to change this, but this cannot be done within 2,5 days. If (and only if) the tools of Sage distribution are capable of generating these files, then the correct solution would be to to remove them complete during patching.
IIRC, I got a message that it would try to generate the files using my autoconf, which is an older version, but it might not work. That's very dangerous.
Also, this means if the tools exist, the file get generated needlessly, slowing the build.
But it will not take 2.5 days to touch them in the spkg-install file.
Dave
comment:131 follow-up: 132 Changed 12 years ago by
Replying to AlexanderDreyer:
Almost all generated files (
configure
,grammar.cc
etc.) again carry the same time stamp as their sources... :(These files can be generated, but there is no need to do so (if you are not a Singular-kernel developer). In fact they are pre-generated in the repo: http://www.singular.uni-kl.de/svn/trunk/ (That's why the time stamps are equal.)
And exactly that can (and did) cause problems; the distributed generated files should be
- newer than their sources,
- up-to-date.
See http://trac.sagemath.org/sage_trac/ticket/9160#comment:22 (and http://trac.sagemath.org/sage_trac/ticket/9160#comment:20 for the latter issue, or the whole ticket...) and also this thread on sage-devel.
Unfortunately we hadn't had the time to add more comments on this to SPKG.txt
, but I expected the people further working on the Singular spkg to be aware of these issues, or take a look at previous tickets related to the spkg. (And I didn't edit the SD 23.5 wiki page, because at that time it didn't seem appropriate, but asked for other people propagating these things.)
The reason for this is that the generators for these files (e.g. specific version of autotools and lex) are not available on all supported platforms.
Of course, there are attempts to change this, but this cannot be done within 2,5 days. If (and only if) the tools of Sage distribution are capable of generating these files, then the correct solution would be to to remove them complete during patching.
<flame> I thought the upstream developers had learned at Sage Days 23.5, but perhaps 3.1.1.4 was just prepared too early... </flame>
This wasn't at topic at SD23.5.
Obviously unfortunately not, see above. I expected it to be one.
comment:132 follow-up: 133 Changed 12 years ago by
Obviously unfortunately not, see above. I expected it to be one.
Neither #9160 wasn't reported upstream, nor the spkg-maintainers were CCed.
comment:133 follow-up: 134 Changed 12 years ago by
Replying to AlexanderDreyer:
Obviously unfortunately not, see above. I expected it to be one.
Neither #9160 wasn't reported upstream, nor the spkg-maintainers were CCed.
Well, the spkg maintainer (and some of the people involved in this ticket here) actually participated the discussion on sage-devel (which referred to #9160).
(And I remember the spkg maintainer telling me "You don't need to cc anybody, we'll look through the tickets anyway..." ;-) )
That's why I had been a bit annoyed or disappointed, and the reason for the flame.
comment:134 Changed 12 years ago by
(And I remember the spkg maintainer telling me "You don't need to cc anybody, we'll look through the tickets anyway..." ;-) )
I guess it was not clear, that this is to be fixed upstream. (It's merely a packaging problem.)
That's why I had been a bit annoyed or disappointed, and the reason for the flame.
No reason to flam the upstream developers. In particular, as the ticket stated, that it had not been reported to them.
Here's the new SPKG:
You also need to apply the patch I am going to upload in a second.