sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.44k stars 480 forks source link

Projective closure and affine patches for algebraic curves #20676

Closed f4f2417a-d7eb-4b3a-be0a-8fc882f53e00 closed 8 years ago

f4f2417a-d7eb-4b3a-be0a-8fc882f53e00 commented 8 years ago

Given a curve in affine space, find its projective closure. Conversely, given a projective curve, find its affine patches with respect to the standard affine coordinate charts.

This could be done more efficiently by using the projective_embedding/affine patches functionality for affine/projective subschemes. Currently the projective_embedding function for affine subschemes doesn't always have the right codomain:

A.<x,y,z> = AffineSpace(3,QQ)
X = A.subscheme([y-x^2,z-x^3])
X.projective_embedding().codomain()

so this should be addressed here, along with creating a projective_closure function for affine subschemes.

Depends on #20697 Depends on #20698

CC: @bhutz @miguelmarco

Component: algebraic geometry

Keywords: gsoc2016

Author: Grayson Jorgenson

Branch/Commit: 91bc630

Reviewer: Ben Hutz

Issue created by migration from https://trac.sagemath.org/ticket/20676

f4f2417a-d7eb-4b3a-be0a-8fc882f53e00 commented 8 years ago

Branch: u/gjorgenson/ticket/20676

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 8 years ago

Commit: b3002ef

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 8 years ago

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

b3002ef20676: First pass at implementation.
miguelmarco commented 8 years ago
comment:3

I have two questions:

1) Are you sure you really need to compute the groebner basis to compute the projective closue? If I am not missing something, just homogenizing the generators, you should get the generators of the projective ideal.

2) I think it would be better to keep the names of the variables when we compute the affine patches (or at least, to have an option to do so). It can be confusing to the user to force a change in the name of the coordinates. An option to choose the names of the new variables in the projective closure would be nice too.

f4f2417a-d7eb-4b3a-be0a-8fc882f53e00 commented 8 years ago
comment:4

Replying to @miguelmarco:

Thanks for looking at this. I think it's not always the case that a given set of generators is sufficient for finding generators for the ideal of the projective closure. One example I have seen referenced in some texts is the twisted cubic in A3 defined by the ideal (y-x^2, z-x^3). Its projective closure is (x^2 − wy, xy − wz, y^2 − xz), but if the given generators of the first ideal are homogenized we get (wy-x^2, zw<sup>2-x</sup>3) instead.

A proof is given in the book ideals, varieties, algorithms that if the generators of a groebner basis (with respect to a graded monomial ordering) for the defining ideal of an affine variety are homogenized, then the result is a set of generators for the ideal of the projective closure, so I tried to emulate that here.

I'll work on getting some flexibility with the variable names.

bhutz commented 8 years ago
comment:5

As Grayson pointed out the twisted cubic is the typical example for needing the gb.

For the variables. Actually it is not a good idea to name them the same as they really aren't the same variables. More importantly, you need to be careful to keep track of the embedding as well, since if you homogenize the affine patch, you would expect to get something back in the same projective space. Take a look at the functionality for projective space and projective subschemes (or in #16838) to see what I mean.

miguelmarco commented 8 years ago
comment:6

Thanks for the correction. I knew I was missing something.

About the names of the variables... right now they might be named the same or not deppending on what were the original names. So I still think that there should be at least on option to let the user decide how are the new variables named: the same way as before, maybe adding a "bar" at the end, or completely new ones.

And bhutz is totally right about the morphisms: that is the important thing to compute (either in this methods or in separated ones).

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 8 years ago

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

39dd21520676: Attempt to keep track of ambient spaces.
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 8 years ago

Changed commit from b3002ef to 39dd215

f4f2417a-d7eb-4b3a-be0a-8fc882f53e00 commented 8 years ago
comment:8

Okay, I actually just finished up an attempt to better manage the creation of ambient spaces in the projective closure/affine patches computation. I tried piggy-backing off of the existing projective_embedding and affine_patch functionality for projective/affine spaces, and it seems to be working so far. Things like:

P.<x,y,z,w> = ProjectiveSpace(QQ,3)
C = Curve([y*z - x^2,w^2 - x*y])
C.ambient_space() == C.affine_patch(0).projective_closure().ambient_space()

and

A.<x,y,z> = AffineSpace(QQ,3)
C = Curve([y-x^2,z-x^3])
C.ambient_space() == C.projective_closure().affine_patch(0).ambient_space()

should now return true. Is this the right way to keep track of everything?

I also found something that seems strange:

A.<x,y,z> = AffineSpace(QQ,3)
C = Curve([y-x^2,z-x^3])
A == C.ambient_space()

returns false. As far as I can tell, this isn't an issue for projective space curves. Also, there seems to be an issue with the class structure of space curves vs plane curves. Right now there is a ProjectiveSpaceCurve_generic class, and a ProjectiveCurve_generic class, with the latter corresponding to plane curves, but the plane curve class does not inherit from the space curve class. Similarly for affine curves. Is there a reason why plane curves shouldn't inherit from the space curve class?

miguelmarco commented 8 years ago
comment:9

I would say it is a bug in Sage. Apparently, schemes don't have rich comparison implemented.

bhutz commented 8 years ago
comment:10

re: variable names.

The way this is handled for general affine and projective subschemes is to allow the user to specify the space that it will be embedded into (or dehomogenized into). This is better functionality than just specifying the variable names, since you can then embedded different curves into the same space.

re: ambient space

A.<x,y,z> = AffineSpace(QQ,3)
C = Curve([y-x^2,z-x^3])
A == C.ambient_space()

I haven't looked at the code, but it looks like Curve assumes you are passing in elements from a polynomial ring and doesn't see if they are actually from a particular affine space and hence creates a new affine space. This is a bug. If you pass in elements of the coordinate ring of an affine space, then the curve should be in that space. It may be that you need to include an optional parameter for the space and/or perhaps have an A.curve() function.

one last thought: Should these curve classes inherit from affine and projective subscheme as well? This could save you a fair amount of code duplication (such as for affine_patch and projective_embedding.

f4f2417a-d7eb-4b3a-be0a-8fc882f53e00 commented 8 years ago

Description changed:

--- 
+++ 
@@ -1 +1,11 @@
 Given a curve in affine space, find its projective closure. Conversely, given a projective curve, find its affine patches with respect to the standard affine coordinate charts.
+
+This could be done more efficiently by using the projective_embedding/affine patches functionality for affine/projective subschemes. Currently the projective_embedding function for affine subschemes doesn't always have the right codomain:
+
+```
+A.<x,y,z> = AffineSpace(3,QQ)
+X = A.subscheme([y-x^2,z-x^3])
+X.projective_embedding().codomain()
+```
+
+so this should be addressed here, along with creating a projective_closure function for affine subschemes.
f4f2417a-d7eb-4b3a-be0a-8fc882f53e00 commented 8 years ago

Dependencies: #20697, #20698

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 8 years ago

Changed commit from 39dd215 to c6fe840

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 8 years ago

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

02349fe20676: projective_embedding revision and projective_closure.
5de70c920697: plane curve classes now inherit from the space curve classes.
ef65aca20676: Merge ticket 20697 into this ticket.
9cbdab320698: revised initialization of generic curves.
b718afa20698: documentation and error formatting fixes.
d4eb8d420698: documentation spacing fixes.
3ffc3aa20676: Merge ticket 20698 into this ticket.
c6fe84020676: implemented curve-level functions.
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 8 years ago

Changed commit from c6fe840 to 0039d7b

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 8 years ago

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

058b90920697: Change class names.
5a5fe9a20697: change "plane_curves" folder name to "curves"
b3e844720697: Merge with latest beta.
3b5fdf820697: Merge in 20698 for access to better initialization of curves.
854159120697: Added documentation and made curve string labels match class names.
b918e7720697: documentation adjustment, and attempt to fix pickle issue.
b38096720697: Missed name changes.
54f4e6220676: Merge again with 20697.
0039d7b20676: Update example strings to match changes from 20697.
bhutz commented 8 years ago

Reviewer: Ben Hutz

bhutz commented 8 years ago
comment:15

R.ideal([R(f) for f in self.defining_polynomials()]) could be replaced with self.defining_ideal()

A.<x,y> = AffineSpace(QQ, 2)
P.<u,v,w>=ProjectiveSpace(QQ,2)
C = Curve([y-x^2], A)
D=C.projective_closure(1,P)
A.<x,y> = AffineSpace(QQ, 2)
P.<u,v,w>=ProjectiveSpace(QQ,2)
C = Curve([u^2-v^2], P)
C.affine_patch(1,A)
A.<x,y,z> = AffineSpace(GF(3), 3)
C = Curve([y-x^2,z-x^3], A)
D=C.projective_closure()
D.affine_patch()==C
bhutz commented 8 years ago
comment:16

err...having a default for affine_patch() doesn't really make sense, so ignore that part.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 8 years ago

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

9262ae520676: changes from review, and spacing fixes.
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 8 years ago

Changed commit from 0039d7b to 9262ae5

f4f2417a-d7eb-4b3a-be0a-8fc882f53e00 commented 8 years ago

Changed keywords from none to gsoc2016

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 8 years ago

Changed commit from 9262ae5 to 91bc630

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 8 years ago

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

91bc63020676: Merge this ticket with 7.3 beta3.
bhutz commented 8 years ago
comment:20

this looks good now

vbraun commented 8 years ago

Changed branch from u/gjorgenson/ticket/20676 to 91bc630