Opened 9 months ago
Closed 9 months ago
#20676 closed enhancement (fixed)
Projective closure and affine patches for algebraic curves
Reported by:  gjorgenson  Owned by:  

Priority:  minor  Milestone:  sage7.3 
Component:  algebraic geometry  Keywords:  gsoc2016 
Cc:  bhutz, mmarco  Merged in:  
Authors:  Grayson Jorgenson  Reviewers:  Ben Hutz 
Report Upstream:  N/A  Work issues:  
Branch:  91bc630 (Commits)  Commit:  91bc630bfa994e8a2dfa587cae62bda2afb475a5 
Dependencies:  #20697, #20698  Stopgaps: 
Description (last modified by )
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([yx^2,zx^3]) X.projective_embedding().codomain()
so this should be addressed here, along with creating a projective_closure function for affine subschemes.
Change History (21)
comment:1 Changed 9 months ago by
 Branch set to u/gjorgenson/ticket/20676
comment:2 Changed 9 months ago by
 Commit set to b3002ef28b0b1f9ae39b1222c446625037420db9
comment:3 followup: ↓ 4 Changed 9 months ago by
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.
comment:4 in reply to: ↑ 3 Changed 9 months ago by
Replying to mmarco:
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 (yx^2, zx^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 (wyx^2, zw^2x^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.
comment:5 Changed 9 months ago by
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.
comment:6 Changed 9 months ago by
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).
comment:7 Changed 9 months ago by
 Commit changed from b3002ef28b0b1f9ae39b1222c446625037420db9 to 39dd215adb96f07bc67854c5debd1f1b49c08032
Branch pushed to git repo; I updated commit sha1. New commits:
39dd215  20676: Attempt to keep track of ambient spaces.

comment:8 Changed 9 months ago by
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 piggybacking 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([yx^2,zx^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([yx^2,zx^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?
comment:9 Changed 9 months ago by
I would say it is a bug in Sage. Apparently, schemes don't have rich comparison implemented.
comment:10 Changed 9 months ago by
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([yx^2,zx^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.
comment:11 Changed 9 months ago by
 Dependencies set to #20697, #20698
 Description modified (diff)
comment:12 Changed 9 months ago by
 Commit changed from 39dd215adb96f07bc67854c5debd1f1b49c08032 to c6fe8401f1ddf7d53f6af060e5db2d9766f05ed3
Branch pushed to git repo; I updated commit sha1. New commits:
02349fe  20676: projective_embedding revision and projective_closure.

5de70c9  20697: plane curve classes now inherit from the space curve classes.

ef65aca  20676: Merge ticket 20697 into this ticket.

9cbdab3  20698: revised initialization of generic curves.

b718afa  20698: documentation and error formatting fixes.

d4eb8d4  20698: documentation spacing fixes.

3ffc3aa  20676: Merge ticket 20698 into this ticket.

c6fe840  20676: implemented curvelevel functions.

comment:13 Changed 9 months ago by
 Commit changed from c6fe8401f1ddf7d53f6af060e5db2d9766f05ed3 to 0039d7b13dad64025ef70542f407bf3af7423e1a
Branch pushed to git repo; I updated commit sha1. New commits:
058b909  20697: Change class names.

5a5fe9a  20697: change "plane_curves" folder name to "curves"

b3e8447  20697: Merge with latest beta.

3b5fdf8  20697: Merge in 20698 for access to better initialization of curves.

8541591  20697: Added documentation and made curve string labels match class names.

b918e77  20697: documentation adjustment, and attempt to fix pickle issue.

b380967  20697: Missed name changes.

54f4e62  20676: Merge again with 20697.

0039d7b  20676: Update example strings to match changes from 20697.

comment:14 Changed 9 months ago by
 Status changed from new to needs_review
comment:15 Changed 9 months ago by
 Reviewers set to Ben Hutz
 Status changed from needs_review to needs_work
 line 1916:
R.ideal([R(f) for f in self.defining_polynomials()])
could be replaced with
self.defining_ideal()
 projective closure should take projective space parameter
A.<x,y> = AffineSpace(QQ, 2) P.<u,v,w>=ProjectiveSpace(QQ,2) C = Curve([yx^2], A) D=C.projective_closure(1,P)
 Affine patch should take affine space parameter
A.<x,y> = AffineSpace(QQ, 2) P.<u,v,w>=ProjectiveSpace(QQ,2) C = Curve([u^2v^2], P) C.affine_patch(1,A)
 Affine patch should have a default patch and that default should match up with the projective closure default
A.<x,y,z> = AffineSpace(GF(3), 3) C = Curve([yx^2,zx^3], A) D=C.projective_closure() D.affine_patch()==C
 you should mention in the doc for projective embedding that the image is the projective closure
comment:16 Changed 9 months ago by
err...having a default for affine_patch() doesn't really make sense, so ignore that part.
comment:17 Changed 9 months ago by
 Commit changed from 0039d7b13dad64025ef70542f407bf3af7423e1a to 9262ae5923e1c14192b87314288902d50e08b104
Branch pushed to git repo; I updated commit sha1. New commits:
9262ae5  20676: changes from review, and spacing fixes.

comment:18 Changed 9 months ago by
 Keywords gsoc2016 added
 Status changed from needs_work to needs_review
comment:19 Changed 9 months ago by
 Commit changed from 9262ae5923e1c14192b87314288902d50e08b104 to 91bc630bfa994e8a2dfa587cae62bda2afb475a5
Branch pushed to git repo; I updated commit sha1. New commits:
91bc630  20676: Merge this ticket with 7.3 beta3.

comment:20 Changed 9 months ago by
 Status changed from needs_review to positive_review
this looks good now
comment:21 Changed 9 months ago by
 Branch changed from u/gjorgenson/ticket/20676 to 91bc630bfa994e8a2dfa587cae62bda2afb475a5
 Resolution set to fixed
 Status changed from positive_review to closed
Branch pushed to git repo; I updated commit sha1. New commits:
20676: First pass at implementation.