Implement kernel_points and inverse_image for elliptic curve hom

[user202729]

Ideally I want to implement .kernel_gens() (and eventually .kernel()) but that requires a lot of framework (subgroup of E.abelian_group(), etc.)

Also it may be possible to use a more efficient algorithm for EllipticCurveHom_composite, but I haven't worked out the math yet. (if you're lucky then taking the inverse image over each successtive φ works, but otherwise…) Currently it takes successive image, but might have bad worst case behavior.

📝 Checklist

• The title is concise and informative.
• The description explains in detail what this PR is about.
• I have linked a relevant issue or discussion.
• I have created tests covering the changes.
• I have updated the documentation and checked the documentation preview.

⌛ Dependencies

[github-actions]

Documentation preview for this PR (built with commit b6176fe; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

[tscrim]

A bunch of minor things.

[tscrim]

Should we explicitly cast Q to the codomain? For example, 2/2 in ZZ but it is not an integer.

[user202729]

__contains__ implementation of EllipticCurve checks isinstance(Q, EllipticCurveModel) and Q.curve() == self if I recalled correctly, so I think this does what it should.

[user202729]

Actually you're right, the current behavior is like this

sage: f.inverse_image((0, 2))
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
...
AttributeError: 'tuple' object has no attribute 'is_zero'

on the other hand,

sage: f.codomain().coerce((0, 2))
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
...
TypeError: no canonical coercion from <class 'tuple'> to Elliptic Curve defined by ...

it's not ideal to directly convert the point, since the zero point in any elliptic curve can be converted into the zero point in any other elliptic curve. I decide to cast it after checking Q in self.codomain().

New behavior:

sage: f.inverse_image((0, 2))
(2 : 3*z2 + 1 : 1)

Or maybe it's a good idea to make conversion not work for the wrong curve, or __contains__ not work for tuple? After all the documentation says

    def __contains__(self, P):
        """
        Return ``True`` if and only if P is a point on the elliptic curve.

        P just has to be something that can be coerced to a point.

see "coerced", not "converted". Yet the implementation converts.

[tscrim]

There isn't necessarily anything wrong with it using conversion to check (yes, the doc should be changed). Well, I'm not sure what the best course of action is here as I don't use this code. John is a very good person to ask about this.

[JohnCremona]

I think that using contains as a test is fine. If E1 and E2 are different curves then "E1(0) in E2" is False.

[JohnCremona]

I noticed this PR just when I was myself computing some inverse images under isogenies, so thanks!

For what I was doing I would need an option extend=True. Perhaps we can mae that a new issue, I would not want to delay this one. It's not an obvious extension since even for the x-coordinates, the roots will in general lie in different fields.

[JohnCremona]

Nothing to add beyond what @tscrim has already said. Good!

[user202729]

I realize there's a bug that if you just call any_root(), you get some x-coordinate that lies in the field but there's no guarantee that the y-coordinate is also in the field (in fact there isn't and previously sometimes the assertion fails). I fallback to roots() instead.

[JohnCremona]

I realize there's a bug that if you just call any_root(), you get some x-coordinate that lies in the field but there's no guarantee that the y-coordinate is also in the field (in fact there isn't and previously sometimes the assertion fails). I fallback to roots() instead.

That is a good catch, which I should have noticed before yesterday's comment.

You might find it helpful to look at the code (which I wrote) for Q.division_points(n) which returns a list of P satisfying nP=Q. That's a specical case of what this function is doing. That code does look at every root x and tries to lift to (x,y). Now if 2Q=0 then both y's are valid solutions, while otherwise at most one is, and you may need to check both to see which one works.

[user202729]

I took a look. The only difference with the current implementation is the check of is 2 torsion, but I think that check is not particularly useful, at best it saves 1-2 evaluation of the morphism at the cost of computing _neg_ to check for 2-torsion. Finding root is likely more expensive anyway.