Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cartesian products of projective space #15448

Closed
bhutz opened this issue Nov 23, 2013 · 39 comments
Closed

cartesian products of projective space #15448

bhutz opened this issue Nov 23, 2013 · 39 comments

Comments

@bhutz
Copy link

bhutz commented Nov 23, 2013

While there is some functionality for cartesian products of projective space in Toric Varieties, it does not mesh well with the functionality of Projective Space. This ticket is to implement a general notion of cartesian products of projective space building off of the projective space functionality.

My main interest in this is dynamical systems on products of projective space and subvarieties of products of projective space.

Component: algebraic geometry

Author: Ben Hutz

Branch: 85d88d0

Reviewer: Volker Braun, Joao Alberto de Faria

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

@bhutz bhutz added this to the sage-6.1 milestone Nov 23, 2013
@bhutz
Copy link
Author

bhutz commented Nov 23, 2013

comment:1

Attachment: trac_15448_projective_product.patch.gz

The current patch is still needs work, mainly lacking in more testing. However, it does implement the majority of what is needed for cartesian products of projective space and their subschemes.

@bhutz bhutz self-assigned this Nov 23, 2013
@nbruin
Copy link
Contributor

nbruin commented Nov 23, 2013

comment:2

Have you tried working with multiply graded rings instead? Computing with bihomogeneous polynomials in
k[x0,x1,x2,y0,y1,y2] tends to be MUCH more efficient than working in the Segre embedding, because the rationality is much better expressed in the bihomogeneous case. Having some functionality to obtain the Segre embedding is of course useful, but I suspect that people having to work in products of projective spaces would be better off with multiply graded coordinate rings most of the time.

@bhutz
Copy link
Author

bhutz commented Nov 23, 2013

comment:3

As far as I'm aware there is no native way to use multiply graded rings in Sage. I'd be happy to look into another way to do things, that's why I've put this up here as 'mostly done'.

Currently, everything but the subscheme dimension computation does just use K[x,y] and ensure multi-homgeneity. For the dimension, it seemed simpler to just use the Segre-embedding since I had already implemented that.

@nbruin
Copy link
Contributor

nbruin commented Nov 23, 2013

comment:4

Replying to @bhutz:

As far as I'm aware there is no native way to use multiply graded rings in Sage. I'd be happy to look into another way to do things, that's why I've put this up here as 'mostly done'.

The arithmetic in a multiply graded ring only depends on the ring structure, so in that respect nothing extra is necessary. You just need to interpret the results properly, i.e., that the ideal
(x-y,u-v) in k[x,y,u,v] describes a point in P1xP1 and not a line in P3. I guess knowing you're working with bihomogeneously generated ideals might affect your choice of term ordering if you need to compute groebner bases.

Basically what I expect is that it's possible to use/adapt the toric variety framework for dynamic purposes as well. I haven't looked into it myself. I'm just sharing my experience that in cases where I needed products of projective varieties, I found using multiple gradings initially daunting but eventually not bad at all and much more convenient and efficient.

@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.1, sage-6.2 Jan 30, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.2, sage-6.3 May 6, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.3, sage-6.4 Aug 10, 2014
@bhutz
Copy link
Author

bhutz commented Sep 3, 2014

Branch: u/bhutz/ticket/15448

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 8, 2014

Commit: 6d82bd9

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 8, 2014

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

6d82bd915448: clean up and error fixing

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 12, 2014

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

1a84aa715448: finish segre embedding

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 12, 2014

Changed commit from 6d82bd9 to 1a84aa7

@bhutz
Copy link
Author

bhutz commented Sep 12, 2014

Changed author from bhutz to Ben Hutz

@vbraun
Copy link
Member

vbraun commented Sep 15, 2014

comment:12

Just found your ticket... I've been working on products of projective spaces, mostly for complete intersections in them. See #16987...

We should think about how to merge the two branches. I would prefer to have a separate directory and not use mixed camelcase / underscores in the global namespace (inside of modules its fine). Which is why I used ProductProjectiveSpaces and sage.schemes.product_projective.

@bhutz
Copy link
Author

bhutz commented Sep 15, 2014

comment:13

hmm...yes I looked through your branch some and it seems like most of our basic premises are the same in how we are implementing the products although I seem to be using the underlying structures from projective_space a little bit more. However, the two branches do seem reasonably compatible.

I have no issue with your naming conventions and separate directory, but I'm not sure the best way to go about merging these two sets of changes. You have more experience on the workflow side of things, so I'm happy to entertain suggestions.

@bhutz
Copy link
Author

bhutz commented Sep 15, 2014

comment:14

After some more looking, the biggest difference is that I've based everything off of projective_space functionality. I really do prefer that approach (this makes affine patches, morphisms, etc. much easier to work with) and I don't think it will have much, if any, affect on how you've done complete intersections.

Here is what I'd like to do in an ideal world:

  • take your first commit and basically move it into my branch. However, I'd still like to build off of projective space. I can integrate any of your additional functionality and merge the differences in our approaches.

  • then you could have your ticket be just the complete intersection portion with this ticket as a dependency. You'll probably have to make a few minor adjustments, but I don't think it will be too much.

As I said above, I'm also open to other suggestions.

@vbraun
Copy link
Member

vbraun commented Sep 15, 2014

Reviewer: Volker Braun

@vbraun
Copy link
Member

vbraun commented Sep 15, 2014

comment:15

Sounds good to me. Can you also adjust your docstrings to always have OUTPUT: on a single line? See http://www.sagemath.org/doc/developer/coding_basics.html#docstring-markup-with-rest-and-sphinx

@bhutz
Copy link
Author

bhutz commented Sep 15, 2014

comment:16

ok. I won't get to it today or tomorrow, but will asap after that.

@sagetrac-jdefaria
Copy link
Mannequin

sagetrac-jdefaria mannequin commented Oct 22, 2014

comment:20

A couple of issues....
The following need better error messages

PP.<x0,x1,y0,y1> = ProductProjectiveSpaces([1,1],QQ)
H = Hom(PP,PP)
f = H([x0*x1*y1^3,x0*x1*y1*y0^2])
Proj.<x0,x1,y0,y1> = ProductProjectiveSpaces(1,QQ)

One should not be able to define a product with only one projective space

Prod.<x0,x1> = ProductProjectiveSpaces([1],QQ)
Prod

Have an extra input for the gens of the new PS, following error highlights why:

PP=ProductProjectiveSpaces([1,2],QQ,'u')
PP.segre_embedding()

Not as big a deal, but when return_embedding == True, it should return only the embedding instead of a tuple

PP=ProductProjectiveSpaces([1,2],QQ,'u')
PP.affine_patch([1,0],True) 

Saw some small spacing issues scattered throughout the documentation as well, make sure to go back and double check

@sagetrac-jdefaria
Copy link
Mannequin

sagetrac-jdefaria mannequin commented Oct 22, 2014

Changed reviewer from Volker Braun to Volker Braun, Joao de Faria

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 2, 2014

Changed commit from 76cbaed to 806cd01

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 2, 2014

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

b982b51Merge branch 'master' into ticket/15448
806cd0115448: merged in beta6 and fixes from review

@bhutz
Copy link
Author

bhutz commented Nov 2, 2014

comment:22

Fixes the issues mentioned. With a quick look through the documentation I wasn't sure what spacing issues you were referring to. Could you be more specific.

@sagetrac-jdefaria
Copy link
Mannequin

sagetrac-jdefaria mannequin commented Nov 5, 2014

comment:23

Looked over the code, everything looks fine now, setting to positive review

@sagetrac-jdefaria
Copy link
Mannequin

sagetrac-jdefaria mannequin commented Nov 5, 2014

comment:24

After speaking to the author, I am switching back to needs review so that the other reviewer can also look over it

@vbraun
Copy link
Member

vbraun commented Nov 5, 2014

comment:25

Looks good to me. Sorry, don't have much time this semester. It would be nice if you could follow the docstring style guide a little bit closer:

http://www.sagemath.org/doc/developer/coding_basics.html#docstring-markup-with-rest-and-sphinx

  • always capitalize EXAMPLES::
  • always newline after OUTPUT: (no "OUTPUT: foo" in a single line), EXAMPLES::, ...

@fchapoton
Copy link
Contributor

Changed branch from u/bhutz/ticket/15448 to public/15448

@fchapoton
Copy link
Contributor

comment:26

I cleaned the doc a little bit.

And also a few things in the code. There remains a pyflakes warning about an used variables "dims" in the morphism.py file.


New commits:

8cf318cMerge branch 'u/bhutz/ticket/15448' into 6.4.rc1
abdb87atrac #15448 doc cleaning patch

@fchapoton
Copy link
Contributor

Changed commit from 806cd01 to abdb87a

@bhutz
Copy link
Author

bhutz commented Nov 8, 2014

Changed branch from public/15448 to u/bhutz/ticket/15448

@bhutz
Copy link
Author

bhutz commented Nov 8, 2014

Changed commit from abdb87a to 85d88d0

@bhutz
Copy link
Author

bhutz commented Nov 8, 2014

comment:28

yes, sorry, those docs could have been cleaner. Thanks Frederic for fixing them. I've now removed the dims variable in morphism.py as it became unnecessary after I implemented _factors(), but didn't get removed at that time.


New commits:

85d88d015448: minor code fix

@sagetrac-jdefaria
Copy link
Mannequin

sagetrac-jdefaria mannequin commented Nov 11, 2014

comment:29

Looked at everything once more, doc test came back clean, setting to positive review

@vbraun
Copy link
Member

vbraun commented Nov 14, 2014

Changed branch from u/bhutz/ticket/15448 to 85d88d0

@jdemeyer
Copy link

Changed reviewer from Volker Braun, Joao de Faria to Volker Braun, Joao Alberto de Faria

@jdemeyer
Copy link

Changed commit from 85d88d0 to none

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants