Computing plane curve models for algebraic curves

[gjorgenson]

Given a generic algebraic curve, compute a plane curve birational to it.

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​65a452c	20790: merge with tickets 20697, 20676
​309eb03	20790: First attempt at implementing projections for projecitve curves.

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​0de1323

20790: implemented projections for affine curves.

[gjorgenson]

I implemented a function to project affine curves that allows the user to specify coordinates to use in the projection. If the given coordinates yield a projection that doesn't have a curve as its image, an error will be raised. I also implemented a plane_projection function that doesn't take any user input but just tries to find two coordinates that will yield a projection that has a plane curve as its image. I think such a projection should always exist, but right now my approach to finding one is just trying all possible projections to two coordinates.

For projective curves, I added some functionality for working with curves defined over finite fields. Now the projection function will test all of the points of the ambient space of a curve over a finite field until it finds a point not on the curve. An error is returned if the curve contains all of the ambient space points.

[mmarco]

A few questions:

• It could make sense in some cases to consider projections even if the image is not a curve (or if it is, but some components could be mapped to points). I think we should allow that, but maybe raising a warning about it.

• In python (and sage), indices start in 0. Shouldn't we follow that approach when selecting the coordinates we project to? MAybe we could also allow a list (or tuple) of variables as input.

• don't modify the input indices inside the function. That could have undesired side effects. Instead, create an internal copy.

• I think you go into too much trouble to define the elimination ideal. Just calling elimination_idealon it should give you the ideal defining the image. Something like this:

sage: A.<x,y,z> = AffineSpace(QQ,3)
sage: C = Curve([y^7 - x^2 + x^3 - 2*z, z^2 - x^7 - y^2], A)
sage: I = C.defining_ideal()
sage: IE = I.elimination_ideal(z)
sage: IE
Ideal (y^14 + 2*x^3*y^7 - 2*x^2*y^7 - 4*x^7 + x^6 - 2*x^5 + x^4 - 4*y^2) of Multivariate Polynomial Ring in x, y, z over Rational Field
sage: A2 = AffineSpace(QQ, 2, (x,y))
sage: H = C.Hom(A2)
sage: phi = H((x,y))
sage: phi
Scheme morphism:
  From: Affine Curve over Rational Field defined by y^7 + x^3 - x^2 - 2*z, -x^7 - y^2 + z^2
  To:   Affine Space of dimension 2 over Rational Field
  Defn: Defined on coordinates by sending (x, y, z) to
        (x, y)
sage: C2 = Curve([A2.coordinate_ring()(i) for i in IE.gens()],A2)
sage: C2
Affine Plane Curve over Rational Field defined by y^14 + 2*x^3*y^7 - 2*x^2*y^7 - 4*x^7 + x^6 - 2*x^5 + x^4 - 4*y^2

should be enough to construct both the morphism and the image curve. Something similar could be done for the projective case.

• Do we want to create new coordinates in the case of affine projection? or would it be better to keep the original ones? Maybe have an option for this?

• I think it would me slightly more pythonic to return a tuple instead of a list.

• In the projective case, ¿would it make sense to have an option to project to smaller subspaces?

• In the plane projection method, instead of breaking the loop, just return L. Also, you could avoid the double loop by just iterating over the Subsets of the coordinates (maybe this is slower, but I think it would make the code easyer to read).

• I am confused by this part of the code in the projective projection:

            l = list(PP.gens())
            for i in range(n+1):
                l[i] = 1
                while(F(l) == 0):
                    l[i] = l[i] + 1

I think that, in general, you could get simpler projections if you start with l[i] = 0 (for instance, most times you will get the projection from point [0:0:...: 0:1].

• In this code, why do you create H in the first time? You redefine it just later without using it:

        H = Hom(PP, PP)
        # only need the first n coordinates of the change of coordinates map
        coords = [PP.gens()[i] - Q[i]/Q[n]*PP.gens()[n] for i in range(n)]
        # create the projection map onto the first n coordinates
        PP2 = ProjectiveSpace(self.base_ring(), n - 1)
        H = Hom(self, PP2)

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​d9d149f

20790: various changes from first review

[mmarco]

Looks good so far.

One addition that would be nice os to allow the user to choose the projection point. It may become useful at some point.

[mmarco]

By the way, instead of

 indices[len(indices) - 1]

you can just use

indices[-1]

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​7e63954

20790: minor changes and option for projective projection to pass a point

[gjorgenson]

Thanks, changes made!

[vbraun]

Reviewer name

[vbraun]

Merge conflict, merge in next beta...

[gjorgenson]

New commits:

​7e1f6cf

20790: merge with 7.3 beta4

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​9828514

20790: merge with 7.3 beta5

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​ee363e4

20790: merge with 7.3 beta6

[chapoton]

You now need to use relative import for constructor:

from .constructor import Curve

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​c69bcd1	20790: removed constructor imports
​16d6207	20790: merge with sage 7.3.rc0

[gjorgenson]

Ok, should be good now

[bhutz]

All tests currently pass, but in looking through the functionality I don't think the 'newvariables' approach is consistent with other parts of sage. the names of the variables does not specify the space. So even if you have two affine spaces of the same dimension, base_ring and variables, they will be isomorphic, but not the same object in Sage. I think it would be better to have the user give the space to project into.

Similarly for the 'same variables' boolean. They really aren't the same variables. I think this just leads to user confusions since now you have two 'x's that mean different things.

Finally, for plane projection, you don't allow this functionality.

I think in all cases the better solution is to allow the user to specify the space, not the names of the variables. Look, for example, at the affine_patch and projective_embedding functionality for subschemes.

Unrelated: the doc tests do not test the input checks nor do they test the 'newvariables' parameter.

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​8785032

20790: changes from review

[gjorgenson]

Ok, the user can now specify an ambient space to project into in each of the projection functions. I also added some input tests.

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​39108a1

20790: added some tests

[gjorgenson]

The projection functions now have tests to show passing an ambient space will ensure the projected curves are defined in that space.

[bhutz]

looks good to me.

[git]

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

​67320bb

Merge branch 'master' into u/gjorgenson/ticket/20790

[gjorgenson]

Fixed a merge conflict with the imports in affine_curve.py, projective_curve.py.

[vbraun]

Merge conflict, probably #21085

[git]

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

​3dd26e7	21085: resolution of singularities implementation, first attempt
​4a6bfbf	21085: some minor changes
​78e1ce2	21085: changes from review
​1cbb355	21085: generalized blowup() to work for any point on a given curve
​d1b1edb	Merge branch 'master' into u/gjorgenson/ticket/21085
​78fdd81	21085: add import back in that was accidentally removed during merge
​50d05de	21285: error in change_ring for affine morphisms
​c6b7a42	21085: merge with ticket 21285 for bug fix
​bebf7b0	21085: minor improvements
​679338b	20790: merge with 21085 to fix conflicts

[gjorgenson]

Ok, fixed the conflict with 21085. I don't think there are any other curve tickets closed after sage 7.4 beta1, so this should be good to go.