Hyperelliptic curve constructor has wrong inheritance #37612
Labels
Milestone
Comments
|
The original dynamic classes were made by @anna-somoza, but I do not know if she is actively working on Sage. I am not an expert in class inheritance so I do not wish to try and fix this. |
|
Direct example of this issue (thanks to @grhkm21) sage: from sage.schemes.hyperelliptic_curves.jacobian_g2 import HyperellipticJacobian_g2
sage: R.<t> = PolynomialRing(GF(next_prime(10^9)))
....: C = HyperellipticCurve(t^5 + t + 1)
sage: type(HyperellipticJacobian_g2(C))
<class 'sage.schemes.hyperelliptic_curves.jacobian_g2.HyperellipticJacobian_g2_with_category'>
sage: HyperellipticJacobian_g2(C).kummer_surface()
Closed subscheme of Projective Space of dimension 3 over Finite Field of size 1000000007 defined by:
X0^4 - 4*X0*X1^3 + 4*X0^2*X1*X2 - 4*X0*X1^2*X2 + 2*X0^2*X2^2 + X2^4 - 4*X0^3*X3 - 2*X0^2*X1*X3 - 2*X1*X2^2*X3 + X1^2*X3^2 - 4*X0*X2*X3^2
sage: C.jacobian().kummer_surface()
---------------------------------------------------------------------------
KeyError Traceback (most recent call last) |
|
OK so I have a bad(??) fix for this. Currently the dynamic class would be made with (for example) class_name = "_".join(cls_name)
cls = dynamic_class(
class_name,
(HyperellipticCurve_g2, HyperellipticCurve_finite_field)
cls=HyperellipticCurve_generic,
doccls=HyperellipticCurve,
)Where both If I set |
vbraun
pushed a commit
to vbraun/sage
that referenced
this issue
Mar 30, 2024
…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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Steps To Reproduce
Expected Behavior
For genus two curves, the base class should be
hyperelliptic_g2which has the functionSo for a genus two curve, we expect a Jacobian of type
HyperellipticJacobian_generic.HyperellipticJacobian_g2_genericbut this does not happen as only thejacobian()function ofHyperellipticCurve_genericis in scope.Actual Behavior
Class construction should set the right inheritance so that the genus two functions work, or the genus two special cases should be handled within the generic class instead.
Additional Information
No response
Environment
Checklist
The text was updated successfully, but these errors were encountered: