Opened 8 years ago
Closed 7 years ago
#13089 closed enhancement (fixed)
Implement weighted projective spaces.
Reported by: | davideklund | Owned by: | AlexGhitza |
---|---|---|---|
Priority: | major | Milestone: | sage-5.3 |
Component: | algebraic geometry | Keywords: | toric |
Cc: | Merged in: | sage-5.3.beta0 | |
Authors: | David Eklund | Reviewers: | Andrey Novoseltsev |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
This ticket aims at an implementation of weighted projective spaces in Sage. This was discussed on sage-devel here: https://groups.google.com/d/topic/sage-devel/rahI8ewn1_c/discussion.
One approach is to implement weighted projective spaces as toric varieties.
Attachments (1)
Change History (21)
comment:1 follow-up: ↓ 3 Changed 8 years ago by
comment:2 Changed 8 years ago by
The attached patch is intended as a starting point to be revised and expanded upon.
It might make sense to give weighted projective spaces a class of their own.
Also, it might be suitable to make them as CPR Fano toric varieties (I think they are all of this type). When making polytopes with LatticePolytope? there seems to be a limit on the dimension (<=6) and I think it might make sense to allow weighted projective spaces of bigger dimension than that.
comment:3 in reply to: ↑ 1 Changed 8 years ago by
I'm not sure what you mean. Are you saying that, when one of the weights is equal to 1 we could make the fan (in a kind of canonical way) and create a toric variety from that, but that the general case is more messy?
The attached patch constructs the fan using quotient lattices, so Sage chooses a basis for the lattice. I like the basis it picks except that (in the present context) one might prefer a different order of the basis vectors.
Replying to novoselt:
At least for spaces that have a variable of weight 1 there should be a conversion to current toric varieties (for general ones in quotient lattices current implementation may be less canonical).
comment:4 Changed 8 years ago by
comment:5 follow-up: ↓ 7 Changed 8 years ago by
Not all WPs are (singular) Fano and general toric varieties are handled by PPL so there are no dimension limits.
If one of the weights is one, e.g. WP(1,1,4,6), then one can take for the fan rays the standard basis plus "negative weights except for one 1", i.e. (-1,-4,-6).
In general you take the standard basis of dimension+1 and quotient by the weights. If this works - great, I just was not sure if there are still problems with fans in quotient lattices in Sage - there were some but we were gradually improving the situation.
comment:6 follow-up: ↓ 8 Changed 8 years ago by
A few more comments:
- Please base the patch on top of #13023.
- It would be nice to call
toric_varieties.WP(1,2,3)
without packing weights into a list or a tuple. - To be more in the spirit of other constructors,
names
keyword parameter should be recognized for coordinate names (and be handled in the same way as others).
comment:7 in reply to: ↑ 5 Changed 8 years ago by
Right now I apply coordinate_vector() to the images in the quotient of the basis in dimension +1. Then I make the fan from this data, so I guess the fan does not live in the quotient lattice but some other lattice of the same rank. This seems to work. Interesting if one can, as I gather from your reply, simply tell Sage to construct the fan (with these and these rays and cones) in the quotient lattice. If there were problems with this feature perhaps a good way of debugging is to use it, here for example.
Replying to novoselt:
Not all WPs are (singular) Fano and general toric varieties are handled by PPL so there are no dimension limits.
If one of the weights is one, e.g. WP(1,1,4,6), then one can take for the fan rays the standard basis plus "negative weights except for one 1", i.e. (-1,-4,-6).
In general you take the standard basis of dimension+1 and quotient by the weights. If this works - great, I just was not sure if there are still problems with fans in quotient lattices in Sage - there were some but we were gradually improving the situation.
comment:8 in reply to: ↑ 6 Changed 8 years ago by
Ok, thanks for pointing this out. I will make these changes.
Replying to novoselt:
A few more comments:
- Please base the patch on top of #13023.
- It would be nice to call
toric_varieties.WP(1,2,3)
without packing weights into a list or a tuple.- To be more in the spirit of other constructors,
names
keyword parameter should be recognized for coordinate names (and be handled in the same way as others).
comment:9 Changed 7 years ago by
New patch attached.
It is based on Sage 5.1.beta5 (so should be consistent with #13023).
The weights need no longer be packed into a tuple or list.
Added keyword 'names' for user defined coordinate names.
comment:10 Changed 7 years ago by
For the patchbot:
Apply trac_13089_weighted_proj_space.patch
comment:11 follow-up: ↓ 12 Changed 7 years ago by
As a side effect of #13183, the handling of quotients is improved and it may be possible to construct fans of WPs in quotient lattices directly.
I also think that the patchbot does not test tickets unless they are in needs_review
status.
comment:12 in reply to: ↑ 11 Changed 7 years ago by
Replying to novoselt:
As a side effect of #13183, the handling of quotients is improved and it may be possible to construct fans of WPs in quotient lattices directly.
I also think that the patchbot does not test tickets unless they are in
needs_review
status.
Ok, cool. I will try to change the patch in the this ticket to take advantage of #13183.
comment:13 Changed 7 years ago by
It seems that quotient lattice are not quite ready for fans. For example, there seems to be a need for a length function for elements of quotients. I did not look at this very carefully though.
In either case, I think it would be good to get the weighted projective space code into the toric varieties library and then simplifications can be made later as functionality of lattice quotients is expanded.
comment:14 Changed 7 years ago by
Attached a new patch with minor changes and some comments in the code.
comment:15 Changed 7 years ago by
- Priority changed from minor to major
- Status changed from new to needs_review
comment:16 Changed 7 years ago by
- Reviewers set to Andrey Novoseltsev
- Status changed from needs_review to needs_work
Thanks for trying to use quotient lattices, I though I took care of length, but apparently not for all cases. Let's move on with WPs as is, just a couple little picks:
Instead of
print "keyword argument %s (value %s) will be ignored" % (key,kw[key])
you should raise an exception, since ideally we want
def WP(self, *q, K=QQ, names="z+"):
and it would. (Showing the traceback that lead to it instead of just the error.)
It is also deprecated in Python to use raise Error, "message"
, rather it should be raise Error("message")
.
comment:17 Changed 7 years ago by
Good points! I have made the suggested changes and replaced the patch.
comment:18 Changed 7 years ago by
- Status changed from needs_work to needs_review
comment:19 Changed 7 years ago by
- Keywords toric added
- Status changed from needs_review to positive_review
comment:20 Changed 7 years ago by
- Merged in set to sage-5.3.beta0
- Resolution set to fixed
- Status changed from positive_review to closed
At least for spaces that have a variable of weight 1 there should be a conversion to current toric varieties (for general ones in quotient lattices current implementation may be less canonical).