#14353 closed enhancement (fixed)
Factor toric morphism into surjective and generically injective
Reported by: | vbraun | Owned by: | AlexGhitza |
---|---|---|---|
Priority: | major | Milestone: | sage-5.11 |
Component: | algebraic geometry | Keywords: | toric, sd48 |
Cc: | novoselt | Merged in: | sage-5.11.beta3 |
Authors: | Volker Braun, Andrey Novoseltsev | Reviewers: | Andrey Novoseltsev, Volker Braun |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
This ticket implements factorization into a surjective, birational, and injective toric morphism.
Apply:
Attachments (6)
Change History (20)
comment:1 Changed 9 years ago by
- Cc novoselt added
- Status changed from new to needs_review
comment:2 Changed 9 years ago by
- Status changed from needs_review to needs_work
comment:3 Changed 9 years ago by
- Status changed from needs_work to needs_review
You are of course right. Fixed now.
comment:4 Changed 9 years ago by
Hey Volker, thanks for opening this ticket!
As I was thinking more about it, toric morphisms naturally factor into THREE: if f: Sigma --> Sigma'
, then we have
Sigma --f_s--> Sigma_s --f_b--> Sigma_i --f_i--> Sigma'
where f_s
is surjection onto the fan which is the image of Sigma, f_i
is injection of Sigma' intersected with the image sublattice, and f_b
is a birational map between the intermediate toric varieties which does not have to be surjective or injective. The map from a single blowup chart into the original variety is precisely f_b
with the other two being identities.
I would like to do the following:
- adjust factoring to return three morphisms instead of two;
- make intermediate fans live in the
image_sublattice
; - name them, perhaps,
domain_fan_image
(under the map of lattices) andcodomain_fan_restriction
(to theimage_sublattice
); - get rid of
restrict_to_image
method as it is misleading, if not wrong: restriction to the image of a toric morphism is not necessarily a toric morphism (it is not for the blow up chart). It does not seem like it is used anywhere but one place and basically it was trying to do the factoring in a not very good way.
I am working on it now on top of your patches, so let me know if you have severe objections!
comment:5 Changed 9 years ago by
I don't think there are particularly good names for the intermediate fans. Instead of trying to invent some name, we can just let a user use domain_fan()
/ image_fan()
on the factored morphism.
We have a choice for Sigma_i
. Just the intersection with the image sublattice will in general contain cones whose preimage under f_b
is empty. We can leave (some of) those out and get a smaller Sigma_i
. E.g. take the composition of the half blowup chart with the embedding C^2-> P^2
. The natural thing to do is probably take this smaller fan. So f_i
would be the embedding C^2->P^2
in the example.
I'm happy to get rid of restrict_to_image
, I was tempted to do that myself....
comment:6 Changed 9 years ago by
Here is some work in progress, does not look good in the patch, but it removes restrict_to_image
in the end and adds factor
instead, diff blends these activities.
No other new methods apart is_birational
which is not proper yet and there are some failures with composition of morphisms - I completely agree that these fans have to be accessed through morphisms, no need for new names.
As for Sigma_i
indeed we have a choice, but I'd rather stick with "the big one" because:
- it makes it independent of the domain fan, which is symmetrical to
Sigma_s
not depending on the codomain fan; - the corresponding variety will be a closed subvariety of the codomain;
- it is easier to implement/faster to compute;
- last but not least - I already have it in the patch ;-)
Changed 9 years ago by
comment:7 Changed 9 years ago by
- Description modified (diff)
- Keywords toric added
- Reviewers set to Andrey Novoseltsev, Volker Braun
- Status changed from needs_review to needs_work
- Work issues set to morphism composition
Hi Volker, I propose this version based on triple factorization for both fan and variety morphisms. I hope the documentation is written clearly and the example chosen gives a good idea what is what. The only remaining problem is that I really want to multiply factors back into the original one. For fan morphisms I just added __mul__
as they are standalone (although it may make sense to change that...), but toric morphisms should follow coercion rules, so implementing the double underscore method as it is done now is not acceptable. And even with it there is an issue of comparing, although it does seem that the result is correct.
Anyway: let me know what you think of composition and if the rest looks OK!
comment:8 Changed 9 years ago by
I think we shouldn't do more than implement the factorization in this ticket, fixing composition of morphisms is another task. Though right now this doctest fails, maybe you want to mark it as # known bug
?
sage -t sage/schemes/toric/morphism.py ********************************************************************** File "sage/schemes/toric/morphism.py", line 544, in sage.schemes.toric.morphism.SchemeMorphism_fan_toric_variety.factor Failed example: prod(phi.factor()) == phi Expected: True Got: False
Changed 9 years ago by
comment:9 Changed 9 years ago by
- Status changed from needs_work to needs_review
- Work issues morphism composition deleted
Well, since composition was actually working and the problem was in comparison, I've added __cmp__
- composing factors to compare with the original is the most natural doctest here. All tests pass now, ready for review!
comment:10 Changed 9 years ago by
Apply trac_14353_fan_morphism_factoring.patch trac_14353_auxiliary_methods.patch trac_14353_toric_morphism_factoring.patch
comment:11 Changed 9 years ago by
- Description modified (diff)
comment:12 Changed 9 years ago by
- Keywords sd48 added
- Status changed from needs_review to positive_review
Positive review after in-persion discussion with Andrey
comment:13 Changed 9 years ago by
- Merged in set to sage-5.11.beta3
- Resolution set to fixed
- Status changed from positive_review to closed
comment:14 Changed 8 years ago by
Follow-up: #14975. Please fix that soon or I will unmerge this ticket.
The support of a fan is in general not a cone at all! Unless I have a severe misunderstanding, it is a set-theoretic union of all its cones, e.g. for the fan generated by 1-cones (1,0) and (0,1) is not a cone. It would be great to have a method for support, but I am not sure how we can represent it at the moment apart from the fan itself.