-
-
Notifications
You must be signed in to change notification settings - Fork 488
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
build/pkgs/singular: Upgrade to 4.3.2p7, reject system Singular without FLINT #35934
Conversation
Apparently there's something wrong with the documentation. |
I was able to build documentation after disabling 3 examples |
08dfe67
to
9a5e54d
Compare
Testing this locally, I am getting
|
Testing this locally with
|
same with |
OTOH, singular 4.3.2p7 gives
|
The 3 failures in --- a/src/sage/schemes/projective/projective_subscheme.py
+++ b/src/sage/schemes/projective/projective_subscheme.py
@@ -1001,7 +1001,7 @@ class AlgebraicScheme_subscheme_projective(AlgebraicScheme_subscheme):
for i in range(n + 1):
J = J + S.ideal(z[-1] * f_S.derivative(z[i]) - z[i + n + 1])
- sat = ff.elim__lib.sat
+ sat = ff.elim__lib.sat_with_exp
max_ideal = S.ideal(z[n + 1: 2 * n + 2])
J_sat_gens = sat(J, max_ideal)[0] Cf. #35980 (and the proper fix to support older singular is as in that PR, ofc). The 24 failures in |
And this fixes the sandpile: --- a/src/sage/sandpiles/sandpile.py
+++ b/src/sage/sandpiles/sandpile.py
@@ -2495,7 +2495,7 @@ class Sandpile(DiGraph):
"""
R = self.ring()
I = self._unsaturated_ideal._singular_()
- self._ideal = R.ideal(I.sat(prod(R.gens())._singular_())[1])
+ self._ideal = R.ideal(I.sat_with_exp(prod(R.gens())._singular_())[1])
def unsaturated_ideal(self):
r""" |
Here's a tentative version of the two fixes above which seems to work ok with both singular 4.3.2p2 (old sat api) and with singular 4.3.2p7 (new sat api). In between versions (p3, p4) are broken. --- a/src/sage/sandpiles/sandpile.py
+++ b/src/sage/sandpiles/sandpile.py
@@ -2493,9 +2493,15 @@ class Sandpile(DiGraph):
sage: '_ideal' in S.__dict__
True
"""
+ from sage.libs.singular.function_factory import ff
+ try:
+ sat = ff.elim__lib.sat_with_exp
+ except NameError:
+ sat = ff.elim__lib.sat
R = self.ring()
- I = self._unsaturated_ideal._singular_()
- self._ideal = R.ideal(I.sat(prod(R.gens())._singular_())[1])
+ I = self._unsaturated_ideal
+ I_sat_gens = sat(I, prod(R.gens()))[0]
+ self._ideal = R.ideal(I_sat_gens)
def unsaturated_ideal(self):
r"""
diff --git a/src/sage/schemes/projective/projective_subscheme.py b/src/sage/schemes/projective/projective_subscheme.py
index e6caf19ba74..afd6484d779 100644
--- a/src/sage/schemes/projective/projective_subscheme.py
+++ b/src/sage/schemes/projective/projective_subscheme.py
@@ -1001,7 +1001,10 @@ class AlgebraicScheme_subscheme_projective(AlgebraicScheme_subscheme):
for i in range(n + 1):
J = J + S.ideal(z[-1] * f_S.derivative(z[i]) - z[i + n + 1])
- sat = ff.elim__lib.sat
+ try:
+ sat = ff.elim__lib.sat_with_exp
+ except NameError:
+ sat = ff.elim__lib.sat
max_ideal = S.ideal(z[n + 1: 2 * n + 2])
J_sat_gens = sat(J, max_ideal)[0] |
@mkoeppe could you try adding the patch from my last comment and with singular 4.3.2p7? |
9a5e54d
to
8f3bc18
Compare
Builds OK on macOS, and |
Documentation preview for this PR (built with commit 8f3bc18; changes) is ready! 🎉 |
Works for me, I tested the patch as-is now in my void linux package. The two reasons I'm not sure about giving positive review are (a) I wrote part of the fix (b) I'm not testing the actual singular spkg as I only build sagelib. OTOH, I am using singular 4.3.2p7 from system (built from unpatched upstream), and I also tested this with 4.3.2p2 (also from system) to check the fallback works. Also I think the CI is testing that this works using the singular spkg, so there's that. |
Yes, it built singular from spkg. More portability tests running at https://github.com/mkoeppe/sage/actions/runs/5934767113 (together with some other upgrades) |
Builds without problems on all platforms. Let's get this in. |
Works for me |
#include <singular/singularconfig.h> | ||
#if !defined(HAVE_FLINT) | ||
# error "Need Singular compiled with FLINT" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any chance that this program compiles?
It seems to be missing #endif
and a main(){}
although I'm not completely sure on the semantics of AC_LANG_PROGRAM
. In any case, this test fails in void linux where singular is built with flint (and /usr/include/singular/singularconfig.h
indeed does contain #define HAVE_FLINT 1
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in #36212, I think
📝 Checklist
⌛ Dependencies