Skip to content

Commit 8e24fa8

Browse files
author
Release Manager
committed
gh-37613: Fix hyperelliptic curve dynamic class construction to allow proper method inheritance While working on some hyperelliptic code, I realised the dynamic class construction had an issue with inheritance for the genus two classes and certain methods which should have been available were not. This is discussed in more detail in the issue #37612 I do not know a *good* fix for this, but I have found a fix which at least allows the functions supposed to be accessed to be accessed. I have added a small doctest to ensure that the method called for genus two curves is bound to the correct class. ## Overview of the fix The original class was constructed as: ```py cls = dynamic_class(class_name, tuple(superclass), cls=HyperellipticCurve_generic, doccls=HyperellipticCurve) ``` Where `superclass` is either an empty list, a list with a single child from: - `HyperellipticCurve_finite_field` - `HyperellipticCurve_rational_field` - `HyperellipticCurve_padic_field` - `HyperellipticCurve_g2` Notice that all four of these classes are children of `HyperellipticCurve_generic`. Or, in the case of the base field being one of the above AND the curve being genus two this list is of the form: ``` [HyperellipticCurve_XXX_field, HyperellipticCurve_g2] ``` In this case, I found that the dynamic class insertion into `cls` meant that the methods shared by one of the "`superclass`" (probably should be called base classes?) and `cls` itself would call the method from `cls` rather than the superclass and so all specialised functions were unavailable, making the genus two optimisations redundant. In my new fix, if `superclass` has a length of zero, I set `cls` to be `HyperellipticCurve_generic`, otherwise `cls` is `None`. This seems to work, but I don't know if this is good practice. Fixes #37612 URL: #37613 Reported by: Giacomo Pope Reviewer(s): Giacomo Pope, Kwankyu Lee
2 parents 7b6eb13 + 02aad62 commit 8e24fa8

File tree

2 files changed

+58
-33
lines changed

2 files changed

+58
-33
lines changed

src/sage/schemes/hyperelliptic_curves/constructor.py

Lines changed: 49 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -92,11 +92,11 @@ def HyperellipticCurve(f, h=0, names=None, PP=None, check_squarefree=True):
9292
sage: HyperellipticCurve(x^8 + 1, x)
9393
Traceback (most recent call last):
9494
...
95-
ValueError: Not a hyperelliptic curve: highly singular at infinity.
95+
ValueError: not a hyperelliptic curve: highly singular at infinity
9696
sage: HyperellipticCurve(x^8 + x^7 + 1, x^4)
9797
Traceback (most recent call last):
9898
...
99-
ValueError: Not a hyperelliptic curve: singularity in the provided affine patch.
99+
ValueError: not a hyperelliptic curve: singularity in the provided affine patch
100100
101101
sage: F.<t> = PowerSeriesRing(FiniteField(2))
102102
sage: P.<x> = PolynomialRing(FractionField(F))
@@ -128,7 +128,7 @@ def HyperellipticCurve(f, h=0, names=None, PP=None, check_squarefree=True):
128128
sage: HyperellipticCurve((x^3-x+2)^2*(x^6-1))
129129
Traceback (most recent call last):
130130
...
131-
ValueError: Not a hyperelliptic curve: singularity in the provided affine patch.
131+
ValueError: not a hyperelliptic curve: singularity in the provided affine patch
132132
133133
sage: HyperellipticCurve((x^3-x+2)^2*(x^6-1), check_squarefree=False)
134134
Hyperelliptic Curve over Finite Field of size 7 defined by
@@ -147,7 +147,7 @@ def HyperellipticCurve(f, h=0, names=None, PP=None, check_squarefree=True):
147147
sage: HyperellipticCurve(f, h)
148148
Traceback (most recent call last):
149149
...
150-
ValueError: Not a hyperelliptic curve: highly singular at infinity.
150+
ValueError: not a hyperelliptic curve: highly singular at infinity
151151
152152
sage: HyperellipticCurve(F)
153153
Hyperelliptic Curve over Rational Field defined by y^2 = x^6 + 1
@@ -160,7 +160,7 @@ def HyperellipticCurve(f, h=0, names=None, PP=None, check_squarefree=True):
160160
sage: HyperellipticCurve(x^5 + t)
161161
Traceback (most recent call last):
162162
...
163-
ValueError: Not a hyperelliptic curve: singularity in the provided affine patch.
163+
ValueError: not a hyperelliptic curve: singularity in the provided affine patch
164164
165165
Input with integer coefficients creates objects with the integers
166166
as base ring, but only checks smoothness over `\QQ`, not over Spec(`\ZZ`).
@@ -203,59 +203,72 @@ def HyperellipticCurve(f, h=0, names=None, PP=None, check_squarefree=True):
203203
"""
204204
# F is the discriminant; use this for the type check
205205
# rather than f and h, one of which might be constant.
206-
F = h**2 + 4*f
206+
F = h**2 + 4 * f
207207
if not isinstance(F, Polynomial):
208-
raise TypeError("Arguments f (= %s) and h (= %s) must be polynomials" % (f, h))
208+
raise TypeError(f"arguments {f = } and {h = } must be polynomials")
209209
P = F.parent()
210210
f = P(f)
211211
h = P(h)
212212
df = f.degree()
213-
dh_2 = 2*h.degree()
213+
dh_2 = 2 * h.degree()
214214
if dh_2 < df:
215-
g = (df-1)//2
215+
g = (df - 1) // 2
216216
else:
217-
g = (dh_2-1)//2
217+
g = (dh_2 - 1) // 2
218218
if check_squarefree:
219219
# Assuming we are working over a field, this checks that after
220220
# resolving the singularity at infinity, we get a smooth double cover
221221
# of P^1.
222222
if P(2) == 0:
223223
# characteristic 2
224224
if h == 0:
225-
raise ValueError("In characteristic 2, argument h (= %s) must be non-zero." % h)
226-
if h[g+1] == 0 and f[2*g+1]**2 == f[2*g+2]*h[g]**2:
227-
raise ValueError("Not a hyperelliptic curve: "
228-
"highly singular at infinity.")
229-
should_be_coprime = [h, f*h.derivative()**2+f.derivative()**2]
225+
raise ValueError(
226+
f"for characteristic 2, argument {h = } must be non-zero"
227+
)
228+
if h[g + 1] == 0 and f[2 * g + 1] ** 2 == f[2 * g + 2] * h[g] ** 2:
229+
raise ValueError(
230+
"not a hyperelliptic curve: highly singular at infinity"
231+
)
232+
should_be_coprime = [h, f * h.derivative() ** 2 + f.derivative() ** 2]
230233
else:
231234
# characteristic not 2
232-
if F.degree() not in [2*g+1, 2*g+2]:
233-
raise ValueError("Not a hyperelliptic curve: "
234-
"highly singular at infinity.")
235+
if F.degree() not in [2 * g + 1, 2 * g + 2]:
236+
raise ValueError(
237+
"not a hyperelliptic curve: highly singular at infinity"
238+
)
235239
should_be_coprime = [F, F.derivative()]
236240
try:
237241
smooth = should_be_coprime[0].gcd(should_be_coprime[1]).degree() == 0
238242
except (AttributeError, NotImplementedError, TypeError):
239243
try:
240244
smooth = should_be_coprime[0].resultant(should_be_coprime[1]) != 0
241245
except (AttributeError, NotImplementedError, TypeError):
242-
raise NotImplementedError("Cannot determine whether "
243-
"polynomials %s have a common root. Use "
244-
"check_squarefree=False to skip this check." %
245-
should_be_coprime)
246+
raise NotImplementedError(
247+
"cannot determine whether "
248+
f"polynomials {should_be_coprime} have a common root, use "
249+
"check_squarefree=False to skip this check"
250+
)
246251
if not smooth:
247-
raise ValueError("Not a hyperelliptic curve: "
248-
"singularity in the provided affine patch.")
252+
raise ValueError(
253+
"not a hyperelliptic curve: singularity in the provided affine patch"
254+
)
249255
R = P.base_ring()
250256
PP = ProjectiveSpace(2, R)
251257
if names is None:
252258
names = ["x", "y"]
253259

254-
superclass = []
260+
bases = []
255261
cls_name = ["HyperellipticCurve"]
256262

263+
# For certain genus we specialise to subclasses with
264+
# optimised methods
257265
genus_classes = {2: HyperellipticCurve_g2}
266+
if g in genus_classes:
267+
bases.append(genus_classes[g])
268+
cls_name.append(f"g{g}")
258269

270+
# For certain base fields, we specialise to subclasses
271+
# with special case methods
259272
def is_FiniteField(x):
260273
return isinstance(x, FiniteField)
261274

@@ -265,19 +278,22 @@ def is_pAdicField(x):
265278
fields = [
266279
("FiniteField", is_FiniteField, HyperellipticCurve_finite_field),
267280
("RationalField", is_RationalField, HyperellipticCurve_rational_field),
268-
("pAdicField", is_pAdicField, HyperellipticCurve_padic_field)]
269-
270-
if g in genus_classes:
271-
superclass.append(genus_classes[g])
272-
cls_name.append("g%s" % g)
281+
("pAdicField", is_pAdicField, HyperellipticCurve_padic_field),
282+
]
273283

274284
for name, test, cls in fields:
275285
if test(R):
276-
superclass.append(cls)
286+
bases.append(cls)
277287
cls_name.append(name)
278288
break
279289

290+
# If no specialised subclasses are identified, we simply use the
291+
# generic class in the class construction
292+
if not bases:
293+
bases = [HyperellipticCurve_generic]
294+
295+
# Dynamically build a class from multiple inheritance. Note that
296+
# all classes we select from are subclasses of HyperellipticCurve_generic
280297
class_name = "_".join(cls_name)
281-
cls = dynamic_class(class_name, tuple(superclass),
282-
HyperellipticCurve_generic, doccls=HyperellipticCurve)
298+
cls = dynamic_class(class_name, tuple(bases), doccls=HyperellipticCurve)
283299
return cls(PP, f, h, names=names, genus=g)

src/sage/schemes/hyperelliptic_curves/hyperelliptic_g2.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,15 @@ def jacobian(self):
4545
sage: f = x^5 - x^4 + 3
4646
sage: HyperellipticCurve(f).jacobian()
4747
Jacobian of Hyperelliptic Curve over Rational Field defined by y^2 = x^5 - x^4 + 3
48+
49+
TESTS:
50+
51+
Ensure that :issue:`37612` is fixed::
52+
53+
sage: R.<x> = QQ[]
54+
sage: f = x^5 - x^4 + 3
55+
sage: type(HyperellipticCurve(f).jacobian())
56+
<class 'sage.schemes.hyperelliptic_curves.jacobian_g2.HyperellipticJacobian_g2_with_category'>
4857
"""
4958
return jacobian_g2.HyperellipticJacobian_g2(self)
5059

0 commit comments

Comments
 (0)