Skip to content
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

Fix hyperelliptic curve dynamic class construction to allow proper method inheritance #37613

Merged
merged 7 commits into from
Mar 31, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 49 additions & 33 deletions src/sage/schemes/hyperelliptic_curves/constructor.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,11 +92,11 @@
sage: HyperellipticCurve(x^8 + 1, x)
Traceback (most recent call last):
...
ValueError: Not a hyperelliptic curve: highly singular at infinity.
ValueError: not a hyperelliptic curve: highly singular at infinity
sage: HyperellipticCurve(x^8 + x^7 + 1, x^4)
Traceback (most recent call last):
...
ValueError: Not a hyperelliptic curve: singularity in the provided affine patch.
ValueError: not a hyperelliptic curve: singularity in the provided affine patch

sage: F.<t> = PowerSeriesRing(FiniteField(2))
sage: P.<x> = PolynomialRing(FractionField(F))
Expand Down Expand Up @@ -128,7 +128,7 @@
sage: HyperellipticCurve((x^3-x+2)^2*(x^6-1))
Traceback (most recent call last):
...
ValueError: Not a hyperelliptic curve: singularity in the provided affine patch.
ValueError: not a hyperelliptic curve: singularity in the provided affine patch

sage: HyperellipticCurve((x^3-x+2)^2*(x^6-1), check_squarefree=False)
Hyperelliptic Curve over Finite Field of size 7 defined by
Expand All @@ -147,7 +147,7 @@
sage: HyperellipticCurve(f, h)
Traceback (most recent call last):
...
ValueError: Not a hyperelliptic curve: highly singular at infinity.
ValueError: not a hyperelliptic curve: highly singular at infinity

sage: HyperellipticCurve(F)
Hyperelliptic Curve over Rational Field defined by y^2 = x^6 + 1
Expand All @@ -160,7 +160,7 @@
sage: HyperellipticCurve(x^5 + t)
Traceback (most recent call last):
...
ValueError: Not a hyperelliptic curve: singularity in the provided affine patch.
ValueError: not a hyperelliptic curve: singularity in the provided affine patch

Input with integer coefficients creates objects with the integers
as base ring, but only checks smoothness over `\QQ`, not over Spec(`\ZZ`).
Expand Down Expand Up @@ -203,59 +203,72 @@
"""
# F is the discriminant; use this for the type check
# rather than f and h, one of which might be constant.
F = h**2 + 4*f
F = h**2 + 4 * f
if not isinstance(F, Polynomial):
raise TypeError("Arguments f (= %s) and h (= %s) must be polynomials" % (f, h))
raise TypeError(f"arguments {f = } and {h = } must be polynomials")

Check warning on line 208 in src/sage/schemes/hyperelliptic_curves/constructor.py

View check run for this annotation

Codecov / codecov/patch

src/sage/schemes/hyperelliptic_curves/constructor.py#L208

Added line #L208 was not covered by tests
P = F.parent()
f = P(f)
h = P(h)
df = f.degree()
dh_2 = 2*h.degree()
dh_2 = 2 * h.degree()
if dh_2 < df:
g = (df-1)//2
g = (df - 1) // 2
else:
g = (dh_2-1)//2
g = (dh_2 - 1) // 2
if check_squarefree:
# Assuming we are working over a field, this checks that after
# resolving the singularity at infinity, we get a smooth double cover
# of P^1.
if P(2) == 0:
# characteristic 2
if h == 0:
raise ValueError("In characteristic 2, argument h (= %s) must be non-zero." % h)
if h[g+1] == 0 and f[2*g+1]**2 == f[2*g+2]*h[g]**2:
raise ValueError("Not a hyperelliptic curve: "
"highly singular at infinity.")
should_be_coprime = [h, f*h.derivative()**2+f.derivative()**2]
raise ValueError(

Check warning on line 225 in src/sage/schemes/hyperelliptic_curves/constructor.py

View check run for this annotation

Codecov / codecov/patch

src/sage/schemes/hyperelliptic_curves/constructor.py#L225

Added line #L225 was not covered by tests
f"for characteristic 2, argument {h = } must be non-zero"
)
if h[g + 1] == 0 and f[2 * g + 1] ** 2 == f[2 * g + 2] * h[g] ** 2:
raise ValueError(
"not a hyperelliptic curve: highly singular at infinity"
)
should_be_coprime = [h, f * h.derivative() ** 2 + f.derivative() ** 2]
else:
# characteristic not 2
if F.degree() not in [2*g+1, 2*g+2]:
raise ValueError("Not a hyperelliptic curve: "
"highly singular at infinity.")
if F.degree() not in [2 * g + 1, 2 * g + 2]:
raise ValueError(
"not a hyperelliptic curve: highly singular at infinity"
)
should_be_coprime = [F, F.derivative()]
try:
smooth = should_be_coprime[0].gcd(should_be_coprime[1]).degree() == 0
except (AttributeError, NotImplementedError, TypeError):
try:
smooth = should_be_coprime[0].resultant(should_be_coprime[1]) != 0
except (AttributeError, NotImplementedError, TypeError):
raise NotImplementedError("Cannot determine whether "
"polynomials %s have a common root. Use "
"check_squarefree=False to skip this check." %
should_be_coprime)
raise NotImplementedError(

Check warning on line 246 in src/sage/schemes/hyperelliptic_curves/constructor.py

View check run for this annotation

Codecov / codecov/patch

src/sage/schemes/hyperelliptic_curves/constructor.py#L246

Added line #L246 was not covered by tests
"cannot determine whether "
f"polynomials {should_be_coprime} have a common root, use "
"check_squarefree=False to skip this check"
)
if not smooth:
raise ValueError("Not a hyperelliptic curve: "
"singularity in the provided affine patch.")
raise ValueError(
"not a hyperelliptic curve: singularity in the provided affine patch"
)
R = P.base_ring()
GiacomoPope marked this conversation as resolved.
Show resolved Hide resolved
PP = ProjectiveSpace(2, R)
if names is None:
names = ["x", "y"]

superclass = []
bases = []
cls_name = ["HyperellipticCurve"]

# For certain genus we specialise to child classes with
# optimised methods
genus_classes = {2: HyperellipticCurve_g2}
if g in genus_classes:
bases.append(genus_classes[g])
cls_name.append(f"g{g}")

GiacomoPope marked this conversation as resolved.
Show resolved Hide resolved
# For certain base fields, we specialise to subclasses
# with special case methods
def is_FiniteField(x):
GiacomoPope marked this conversation as resolved.
Show resolved Hide resolved
return isinstance(x, FiniteField)

Expand All @@ -265,19 +278,22 @@
fields = [
("FiniteField", is_FiniteField, HyperellipticCurve_finite_field),
("RationalField", is_RationalField, HyperellipticCurve_rational_field),
("pAdicField", is_pAdicField, HyperellipticCurve_padic_field)]

if g in genus_classes:
superclass.append(genus_classes[g])
cls_name.append("g%s" % g)
("pAdicField", is_pAdicField, HyperellipticCurve_padic_field),
]

for name, test, cls in fields:
if test(R):
superclass.append(cls)
bases.append(cls)
cls_name.append(name)
break

# If no specialised child class was found, we simply use the
# generic class in the class construction
if not bases:
GiacomoPope marked this conversation as resolved.
Show resolved Hide resolved
bases = [HyperellipticCurve_generic]

# Dynamically build a class from multiple inheritance. Note that
# all classes we slect from are children of HyperellipticCurve_generic
class_name = "_".join(cls_name)
GiacomoPope marked this conversation as resolved.
Show resolved Hide resolved
cls = dynamic_class(class_name, tuple(superclass),
HyperellipticCurve_generic, doccls=HyperellipticCurve)
cls = dynamic_class(class_name, tuple(bases), doccls=HyperellipticCurve)
return cls(PP, f, h, names=names, genus=g)
kwankyu marked this conversation as resolved.
Show resolved Hide resolved
9 changes: 9 additions & 0 deletions src/sage/schemes/hyperelliptic_curves/hyperelliptic_g2.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,15 @@ def jacobian(self):
sage: f = x^5 - x^4 + 3
sage: HyperellipticCurve(f).jacobian()
Jacobian of Hyperelliptic Curve over Rational Field defined by y^2 = x^5 - x^4 + 3

TESTS:

Ensure that :issue:`37612` is fixed::

sage: R.<x> = QQ[]
sage: f = x^5 - x^4 + 3
sage: type(HyperellipticCurve(f).jacobian())
<class 'sage.schemes.hyperelliptic_curves.jacobian_g2.HyperellipticJacobian_g2_with_category'>
"""
GiacomoPope marked this conversation as resolved.
Show resolved Hide resolved
return jacobian_g2.HyperellipticJacobian_g2(self)

Expand Down
Loading