Skip to content

Commit

Permalink
sagemathgh-37613: Fix hyperelliptic curve dynamic class construction …
Browse files Browse the repository at this point in the history
…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
sagemath#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 sagemath#37612
    
URL: sagemath#37613
Reported by: Giacomo Pope
Reviewer(s): Giacomo Pope, Kwankyu Lee
  • Loading branch information
Release Manager committed Mar 29, 2024
2 parents 7b6eb13 + 02aad62 commit 8e24fa8
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 33 deletions.
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 @@ def HyperellipticCurve(f, h=0, names=None, PP=None, check_squarefree=True):
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 @@ def HyperellipticCurve(f, h=0, names=None, PP=None, check_squarefree=True):
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 @@ def HyperellipticCurve(f, h=0, names=None, PP=None, check_squarefree=True):
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 @@ def HyperellipticCurve(f, h=0, names=None, PP=None, check_squarefree=True):
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 @@ def HyperellipticCurve(f, h=0, names=None, PP=None, check_squarefree=True):
"""
# 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")
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(
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(
"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()
PP = ProjectiveSpace(2, R)
if names is None:
names = ["x", "y"]

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

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

# For certain base fields, we specialise to subclasses
# with special case methods
def is_FiniteField(x):
return isinstance(x, FiniteField)

Expand All @@ -265,19 +278,22 @@ def is_pAdicField(x):
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 subclasses are identified, we simply use the
# generic class in the class construction
if not bases:
bases = [HyperellipticCurve_generic]

# Dynamically build a class from multiple inheritance. Note that
# all classes we select from are subclasses of HyperellipticCurve_generic
class_name = "_".join(cls_name)
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)
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'>
"""
return jacobian_g2.HyperellipticJacobian_g2(self)

Expand Down

0 comments on commit 8e24fa8

Please sign in to comment.