edgarcosta / endomorphisms

Rigorous computation of the endomorphism ring of a Jacobian
GNU General Public License v2.0
10 stars 8 forks source link

HeuristicEndomorphismAlgebra not working under CHIMP #30

Closed AndrewVSutherland closed 4 years ago

AndrewVSutherland commented 4 years ago

On a clean magma install of V2.25-1 attaching CHIMP.spec (and nothing else) and calling HeuristicEndomorphismAlgebra on a genus 2 curve fails (iota not assigned in PeriodMatrix):

Magma V2.25-1     Fri Jan 24 2020 12:57:51 on pop-os   [Seed = 1797159272]

+-------------------------------------------------------------------+
|       This copy of Magma has been made available through a        |
|                   generous initiative of the                      |
|                                                                   |
|                         Simons Foundation                         |
|                                                                   |
| covering U.S. Colleges, Universities, Nonprofit Research entities,|
|               and their students, faculty, and staff              |
+-------------------------------------------------------------------+

Type ? for help.  Type <Ctrl>-D to quit.
> AttachSpec("CHIMP.spec");
> C:=HyperellipticCurve(PolynomialRing(Rationals())![1,1,0,0,0,1]);
> HeuristicEndomorphismAlgebra(C);

HeuristicEndomorphismAlgebra(
    X: X
)
GeometricEndomorphismRepresentation(
    X: X
)
PeriodMatrix(
    X: X
)
In file "/home/drew/Dropbox/github/CHIMP/endomorphisms/endomorphisms/magma/heur\
istic/Periods.m", line 81, column 33:
>> F := BaseRing(X); CC := Parent(F`iota);
                                   ^
Runtime error in `: Attribute 'iota' for this structure is valid but not 
assigned
JRSijsling commented 4 years ago

For me, replacing Rationals() with RationalsExtra() works. Long story short: One needs an infinite place for every number field, and the package emphasizes this in this way. Default precision is 100.

AndrewVSutherland commented 4 years ago

Would it be too much to ask to add the line

if BaseRing(C) eq Rationals() then C:=ChangeRing(C,RationalsExtra()); end if;

to the endomorphism functions (display a warning if you like)? I used to have my own wrappers for everything which took care of this, (with an optional Precision parameter you could pass in), as well as a lot of other things, which is why I forgot, but with your new interface (which I like a lot better!) I wanted to try calling your functions directly.

In any case using RationalsExtra() just exposes the next problem:

Magma V2.25-3     Mon Jan 27 2020 16:34:18 on thelio   [Seed = 3736066014]

+-------------------------------------------------------------------+
|       This copy of Magma has been made available through a        |
|                   generous initiative of the                      |
|                                                                   |
|                         Simons Foundation                         |
|                                                                   |
| covering U.S. Colleges, Universities, Nonprofit Research entities,|
|               and their students, faculty, and staff              |
+-------------------------------------------------------------------+

Type ? for help.  Type <Ctrl>-D to quit.
> AttachSpec("CHIMP.spec");
> C:=HyperellipticCurve(PolynomialRing(RationalsExtra())![1,1,0,0,0,1]);
> HeuristicEndomorphismAlgebra(C);

HeuristicEndomorphismAlgebra(
    X: Hyperelliptic Curve defined by y^2 = x^5 + x + 1 over Ration...
)
EndomorphismStructure(
    EndoRep: [ [* [1 0] [0 1],  [1 0 0 0] [0 1 0 0] [0 0 1 0] [0 0 0 1] *...
)
EndomorphismAlgebraZZ(
    C: Associative Algebra of dimension 1 with base ring Rational F...,
    GensC: [ (1) ]
)
In file "/home/drew/Dropbox/github/CHIMP/endomorphisms/endomorphisms/magma/heur\
istic/Structure.m", line 295, column 58:
>> DOC := Discriminant(OC); DOM := Discriminant(MaximalOrder(C));
                                                            ^
Runtime error in 'MaximalOrder': Ambiguous signature match

Matching signatures:
   (A::AlgAssV[FldRat]) -> AlgAssVOrd
   (A::AlgAss) -> AlgAss
Argument types given: AlgAss[FldRat]
JRSijsling commented 4 years ago

Actually RationalsExtra is not a new class at the moment. Instead I just put some bells and whistles on the existing classes. Yes, that is stupid, and the code should rather declare new types, after which your fix is logical and natural. I was too lazy to do this the first time around and will try this soon.

The second bug comes from elsewhere: I do not get it in version 2.21-2.

edgarcosta commented 4 years ago

The second bug is a CHIMP bug. It comes from David Kohel's package echidna. I will fix it asap

edgarcosta commented 4 years ago

The second bug has been fixed.

JRSijsling commented 4 years ago

The first issue has been resolved in the new push.

edgarcosta commented 4 years ago

Awesome, the changes are now available under CHIMP