Opened 2 years ago

Closed 2 years ago

# Projective closure and affine patches for algebraic curves

Reported by: Owned by: gjorgenson minor sage-7.3 algebraic geometry gsoc2016 bhutz, mmarco Grayson Jorgenson Ben Hutz N/A 91bc630 (Commits) 91bc630bfa994e8a2dfa587cae62bda2afb475a5 #20697, #20698

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.

### comment:1 Changed 2 years ago by gjorgenson

• Branch set to u/gjorgenson/ticket/20676

### comment:2 Changed 2 years ago by git

• Commit set to b3002ef28b0b1f9ae39b1222c446625037420db9

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

 ​b3002ef `20676: First pass at implementation.`

### comment:3 follow-up: ↓ 4 Changed 2 years ago by mmarco

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 2 years ago by gjorgenson

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^2-x^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 2 years ago by bhutz

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 2 years ago by mmarco

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 2 years ago by git

• 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 2 years ago by gjorgenson

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?

### comment:9 Changed 2 years ago by mmarco

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

### comment:10 Changed 2 years ago by bhutz

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.

### comment:11 Changed 2 years ago by gjorgenson

• Dependencies set to #20697, #20698
• Description modified (diff)

### comment:12 Changed 2 years ago by git

• 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 curve-level functions.`

### comment:13 Changed 2 years ago by git

• 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 2 years ago by gjorgenson

• Status changed from new to needs_review

### comment:15 Changed 2 years ago by bhutz

• 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([y-x^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^2-v^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([y-x^2,z-x^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 2 years ago by bhutz

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

### comment:17 Changed 2 years ago by git

• 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 2 years ago by gjorgenson

• Status changed from needs_work to needs_review

### comment:19 Changed 2 years ago by git

• 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 2 years ago by bhutz

• Status changed from needs_review to positive_review

this looks good now

### comment:21 Changed 2 years ago by vbraun

• Branch changed from u/gjorgenson/ticket/20676 to 91bc630bfa994e8a2dfa587cae62bda2afb475a5
• Resolution set to fixed
• Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.