Opened 4 years ago

Closed 3 years ago

Last modified 3 years ago

#15448 closed enhancement (fixed)

cartesian products of projective space

Reported by: bhutz Owned by: bhutz
Priority: major Milestone: sage-6.4
Component: algebraic geometry Keywords:
Cc: Merged in:
Authors: Ben Hutz Reviewers: Volker Braun, Joao Alberto de Faria
Report Upstream: N/A Work issues:
Branch: 85d88d0 (Commits) Commit:
Dependencies: Stopgaps:

Description

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.

Attachments (1)

trac_15448_projective_product.patch (53.1 KB) - added by bhutz 4 years ago.

Download all attachments as: .zip

Change History (32)

Changed 4 years ago by bhutz

comment:1 Changed 4 years ago by bhutz

  • Owner changed from (none) to bhutz

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.

comment:2 Changed 4 years ago by nbruin

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.

comment:3 follow-up: Changed 4 years ago by 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'.

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.

comment:4 in reply to: ↑ 3 Changed 4 years ago by nbruin

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.

comment:5 Changed 4 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:6 Changed 3 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:7 Changed 3 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:8 Changed 3 years ago by bhutz

  • Branch set to u/bhutz/ticket/15448
  • Created changed from 11/23/13 16:29:10 to 11/23/13 16:29:10
  • Modified changed from 08/10/14 16:51:03 to 08/10/14 16:51:03

comment:9 Changed 3 years ago by git

  • Commit set to 6d82bd9e56dfb3947907cc0f3c73b980c237d8d0

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

6d82bd915448: clean up and error fixing

comment:10 Changed 3 years ago by git

  • Commit changed from 6d82bd9e56dfb3947907cc0f3c73b980c237d8d0 to 1a84aa7bd51de2ecf645d0e293398b14ef1df3e7

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

1a84aa715448: finish segre embedding

comment:11 Changed 3 years ago by bhutz

  • Authors changed from bhutz to Ben Hutz
  • Status changed from new to needs_review

comment:12 Changed 3 years ago by vbraun

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.

comment:13 Changed 3 years ago by bhutz

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.

comment:14 Changed 3 years ago by bhutz

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.

comment:15 Changed 3 years ago by vbraun

  • Reviewers set to Volker Braun

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

comment:16 Changed 3 years ago by bhutz

  • Status changed from needs_review to needs_work

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

comment:17 Changed 3 years ago by git

  • Commit changed from 1a84aa7bd51de2ecf645d0e293398b14ef1df3e7 to 298ae7c225436b0fcaf97fc24ec2dc81d0021ff7

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

298ae7cchange naming and dir structure to match Trac #16987

comment:18 Changed 3 years ago by bhutz

  • Status changed from needs_work to needs_review

All right. See what you think of this version. I changed to your name and directory structure and added some of your functions, but not all of them, as some are probably not needed with how I've implemented the product.

comment:19 Changed 3 years ago by git

  • Commit changed from 298ae7c225436b0fcaf97fc24ec2dc81d0021ff7 to 76cbaed507ba63dff8dcd7060e58bb83792f725f

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

76cbaedminor fix in comments

comment:20 Changed 3 years ago by jdefaria

  • Reviewers changed from Volker Braun to Volker Braun, Joao de Faria
  • Status changed from needs_review to needs_work

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

comment:21 Changed 3 years ago by git

  • Commit changed from 76cbaed507ba63dff8dcd7060e58bb83792f725f to 806cd01187b03c6dbfb58b927d3f1999bbb72c7e

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

comment:22 Changed 3 years ago by bhutz

  • Status changed from needs_work to needs_review

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.

comment:23 Changed 3 years ago by jdefaria

  • Status changed from needs_review to positive_review

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

comment:24 Changed 3 years ago by jdefaria

  • Status changed from positive_review to needs_review

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

comment:25 Changed 3 years ago by vbraun

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::, ...

comment:26 Changed 3 years ago by chapoton

  • Branch changed from u/bhutz/ticket/15448 to public/15448
  • Commit changed from 806cd01187b03c6dbfb58b927d3f1999bbb72c7e to abdb87a9f664f524d5b361c895ddf8ac5f47ceac

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

comment:27 Changed 3 years ago by bhutz

  • Branch changed from public/15448 to u/bhutz/ticket/15448
  • Modified changed from 11/05/14 20:38:17 to 11/05/14 20:38:17

comment:28 Changed 3 years ago by bhutz

  • Commit changed from abdb87a9f664f524d5b361c895ddf8ac5f47ceac to 85d88d0f66567f9e35e0496b2e7bbf5eb4ebbd9a

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

comment:29 Changed 3 years ago by jdefaria

  • Status changed from needs_review to positive_review

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

comment:30 Changed 3 years ago by vbraun

  • Branch changed from u/bhutz/ticket/15448 to 85d88d0f66567f9e35e0496b2e7bbf5eb4ebbd9a
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:31 Changed 3 years ago by jdemeyer

  • Commit 85d88d0f66567f9e35e0496b2e7bbf5eb4ebbd9a deleted
  • Reviewers changed from Volker Braun, Joao de Faria to Volker Braun, Joao Alberto de Faria
Note: See TracTickets for help on using tickets.