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)

trac_13089_weighted_proj_space.patch (4.3 KB) - added by davideklund 7 years ago.
Implements weighted projective spaces as toric varieties.

Download all attachments as: .zip

Change History (21)

comment:1 follow-up: Changed 8 years ago by 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:2 Changed 8 years ago by davideklund

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 davideklund

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 davideklund

  • Authors set to David Eklund

comment:5 follow-up: Changed 8 years ago by 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:6 follow-up: Changed 8 years ago by 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:7 in reply to: ↑ 5 Changed 8 years ago by davideklund

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 davideklund

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 davideklund

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 davideklund

For the patchbot:

Apply trac_13089_weighted_proj_space.patch

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

comment:12 in reply to: ↑ 11 Changed 7 years ago by davideklund

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 davideklund

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 davideklund

Attached a new patch with minor changes and some comments in the code.

comment:15 Changed 7 years ago by davideklund

  • Priority changed from minor to major
  • Status changed from new to needs_review

comment:16 Changed 7 years ago by novoselt

  • 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").

Changed 7 years ago by davideklund

Implements weighted projective spaces as toric varieties.

comment:17 Changed 7 years ago by davideklund

Good points! I have made the suggested changes and replaced the patch.

comment:18 Changed 7 years ago by davideklund

  • Status changed from needs_work to needs_review

comment:19 Changed 7 years ago by novoselt

  • Keywords toric added
  • Status changed from needs_review to positive_review

comment:20 Changed 7 years ago by jdemeyer

  • Merged in set to sage-5.3.beta0
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.