Closed malb closed 14 years ago
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?
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
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
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
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.
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.
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
Description changed:
---
+++
@@ -1 +1,3 @@
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.
+
+
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.
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
Based on Martin's singular-3-1-1-3 package, but with improved error checking.
Attachment: 8059-improved-error-checking.patch.gz
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
Attachment: simplify-singular.patch.gz
Simplify the building of singular
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.
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
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
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
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
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
Changed upstream from Fixed upstream, in a later stable release. to Reported upstream. Little or no feedback.
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
Attachment: singular-3-1-1-3.patch.gz
fixed many SIGSEGVs
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]
Attachment: singular-3-1-1-4.patch.gz
The Singular team cut us a new release which
The SPKG is at
http://sage.math.washington.edu/home/malb/spkgs/singular-3-1-1-4.spkg
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
Reviewer: David Kirkby
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
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.
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?
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
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.spkg
David, 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
on bsd.math.washington.edu with MAKE=make -j8
real 6m12.144s
user 8m16.609s
sys 1m5.636s
REFEREE REPORT:
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:
Hey, I'm wrong -- you got rid of the .cvsignores!
This gets a positive so long as it builds on stuff.
Hey, I'm wrong -- you got rid of the .cvsignores!
This gets a positive so long as it builds on stuff.
All tests pass on bsd.math
William just finished compiling this SPKG on Cygwin.
Amazingly, this fully works on Cygwin!!
Also builds on Itanium Linux (iras)
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"
Are there any Singular self-tests? There is no spkg-check
file, so these are not run if SAGE_CHECK is set to yes.
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
Please see #9583
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.
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
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.)
More specifically, I have failures on
Maybe it's not compatible with #1396?
Hi jhpalmieri, can you give me instructions how to reproduce this problem, i.e. what patches you've merged etc.?
@jhpalmieri The spkg does need singular-3-1-1-4.patch to be applied to the Sage library.
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.
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...
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
Replying to @sagetrac-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.
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:
devel/sage
Upstream: Reported upstream. Little or no feedback.
CC: @sagetrac-PolyBoRi @sagetrac-drkirkby @aghitza @jaapspies @kiwifb @alexanderdreyer
Component: packages: standard
Author: Martin Albrecht
Reviewer: David Kirkby, Simon King, William Stein, John Palmieri
Merged: sage-4.5.3.alpha1
Issue created by migration from https://trac.sagemath.org/ticket/8059