Opened 10 years ago
Closed 10 years ago
#11385 closed enhancement (fixed)
Orbit closure as toric variety
Reported by: | vbraun | Owned by: | AlexGhitza |
---|---|---|---|
Priority: | major | Milestone: | sage-4.7.2 |
Component: | algebraic geometry | Keywords: | sd31 |
Cc: | novoselt | Merged in: | sage-4.7.2.alpha3 |
Authors: | Volker Braun, Andrey Novoseltsev | Reviewers: | Andrey Novoseltsev, Volker Braun |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
The closure of the torus orbit associated to a cone of the fan is again a toric variety. This patch implements a method toric_variety.orbit_closure(cone)
to construct this toric variety.
Apply:
Attachments (2)
Change History (13)
comment:1 Changed 10 years ago by
- Cc novoselt added
- Status changed from new to needs_review
comment:2 Changed 10 years ago by
- Reviewers set to Andrey Novoseltsev
comment:3 Changed 10 years ago by
- Keywords sd31 added
- Status changed from needs_review to needs_work
- Work issues set to documentation clarification
comment:4 Changed 10 years ago by
- Status changed from needs_work to needs_review
I've added some documentation and a reference to [CLS].
comment:5 Changed 10 years ago by
Note that this is my first day in sage package reviewing, so take this with a grain of salt. However, I did manage to apply another patch correctly before this one.
When I hg.apply() the patch, I get the following error :
hg_sage.apply("/home/simon/sage/devel/sage-test/patches/trac_11385_orbit_cl> cd "/home/simon/sage/devel/sage" && hg status cd "/home/simon/sage/devel/sage" && hg status cd "/home/simon/sage/devel/sage" && hg import "/home/simon/sage/devel/sage-test/patches/trac_11385_orbit_closure_as_toric_variety.patch" applying /home/simon/sage/devel/sage-test/patches/trac_11385_orbit_closure_as_toric_variety.patch internal patcher failed please report details to http://mercurial.selenic.com/bts/ or mercurial@selenic.com /home/simon/sage/local/bin/patch: **** Only garbage was found in the patch input. abort: patch command failed: exited with status 512
comment:6 Changed 10 years ago by
Thanks for having a look! Instead of apply
, its probably better to use import_patch
like this:
sage: hg_sage.import_patch('http://trac.sagemath.org/sage_trac/raw-attachment/ticket/11385/trac_11385_orbit_closure_as_toric_variety.patch')
You probably downloaded the html-formatted patch instead of the raw patch file.
If you want to get more serious about Sage development then you should read about mercurial queues, though. This is a much better way to handle multiple patches.
comment:7 Changed 10 years ago by
- Work issues documentation clarification deleted
For the record - I have not abandoned this patch ;-) I would be happier if the default was to construct the fan of the orbit closure in the honest quotient lattice. I suspect Volker has not done it this way because it just does not work. Well, I am trying to make it work, since it will be yet another step in the direction of the full support of quotients and sublattices. I think they should be freely usable everywhere where lattices can be used, because, well, they are lattices as well ;-)
comment:8 Changed 10 years ago by
- Milestone changed from sage-4.7.1 to sage-4.7.2
Making it work properly with quotient lattices is a noble goal, of course.
Another question is where to put the method to restrict line bundles from the ambient to the orbit closure. Because of the quotient lattice it is not literally pull-back by a toric morphism. I tend to think that ToricOrbitClosure
should derive from ToricVariety
and add a restrict_divisor()
method.
Though as far as this ticket is concerned, I think we all agree that there should be a .orbit_closure(cone)
method for toric varieties that returns a toric variety. So maybe we should merge this ticket before improving the presentation of the resulting toric variety.
Changed 10 years ago by
comment:9 Changed 10 years ago by
- Description modified (diff)
- Reviewers changed from Andrey Novoseltsev to Andrey Novoseltsev, Volker Braun
Ideally the toric variety of the orbit closure should come with the canonical embedding, even though it does not come from a fan morphism. So I think it would be better to have the restriction directly in the toric variety class.
The attached patch contains bits of code that move us closer to supporting quotient lattices, so I propose to add them - I wrote them while making the commented code work. It still does not work for cones, so I agree to leave the rest of necessary changes for the future. I also changed the docstrings quite a bit, hopefully for the best ;-)
If you are OK with the new stuff, please switch to positive review.
comment:11 Changed 10 years ago by
- Merged in set to sage-4.7.2.alpha3
- Resolution set to fixed
- Status changed from positive_review to closed
Um, I don't really understand the documentation of the first (private) function. Could you please expand/rewrite it a bit?-) In particular the INPUT section is a bit confusing given that there are two parameters and one of them is already called cone.
As a possible optimization
can be replaced with
where
discard_faces
is added to the fan module by #11200.