Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Interface for blowups and blowdown morphisms #2780

Merged
merged 2 commits into from
Oct 9, 2023

Conversation

HechtiDerLachs
Copy link
Collaborator

This is a draft to fix an interface for blowups in general. Please let me know your thoughts, ideas, and objections here.

CC: @afkafkafk13 , @HereAround @apturner .

As this is derived from #2779 , you only need to consider the last commit.

@HechtiDerLachs HechtiDerLachs marked this pull request as draft September 7, 2023 15:54
@HechtiDerLachs
Copy link
Collaborator Author

@HereAround : I would like to introduce a new type for toric blowups via fan subdivision. Up to now, the blow_up method returns a usual toric_morphism. But this morphism does not know about its exceptional_divisor, its center, etc., so it is not yet suited to implement the interface formulated here. A new type could store this extra information and provide the required entry points for the entailing functionality. I will try to work out a prototype and push it here for discussion.

@afkafkafk13
Copy link
Collaborator

afkafkafk13 commented Sep 7, 2023

This looks good to me. Do you expect weighted blow ups to comply with these interface settings as well sometime in the farther future, when they are implemented?
Edit: no it is not sufficiently general for this, weighted blow-ups live in a stacky world!

@HechtiDerLachs
Copy link
Collaborator Author

My understanding of weighted blowups does not go very deep as for now. But yes: I hope that they will also fit in here. The natural next step, however, would be sequences of simple blowups as they occur in a resolution process. That would be my first question here: Do you think this can accommodate a resolution of singularities?

@afkafkafk13
Copy link
Collaborator

Basically yes, the sequence of blow-ups in a Hironaka-style resolution is just a means to construct the proper birational covered-scheme morphism which is the resolution of singularities. Only the command center is not straight forward and I would not make it part of the mandatory set of commands to implement.
(In this case, we also need a means to access the individual steps)

Let us now think about a Lipman-style resolution of surfaces: Here we have two different kinds of birational morphisms, namely blowups and normalization. Then we would need a more general type of proper birational morphism of schemes. In this case, it does not make sense to talk about exceptional divisors or exceptional loci either. so I would say that we ignore the case for now.

If we think about a weighted/log-weighted resolution, the design fits again, but the morphisms will not be morphisms of schemes, but of stacks.

experimental/Schemes/ToricBlowups.jl Outdated Show resolved Hide resolved
experimental/Schemes/ToricBlowups.jl Outdated Show resolved Hide resolved
experimental/Schemes/ToricBlowups.jl Outdated Show resolved Hide resolved
experimental/Schemes/ToricBlowups.jl Outdated Show resolved Hide resolved
test/AlgebraicGeometry/ToricVarieties/toric_schemes.jl Outdated Show resolved Hide resolved
@HereAround
Copy link
Member

To keep working on this and after talking to @HechtiDerLachs, I have just squashed the previous commits and rebased this on the current master.

@HereAround
Copy link
Member

Another attempt on the rebase and squash... hopefully it works this time.

@HereAround
Copy link
Member

Pushed a bit of clean-up, but more to come...

@HechtiDerLachs
Copy link
Collaborator Author

The failing tests in ubuntu-latest do not seem to be our fault. @HereAround : Can we do something about the failing doctests?

@HereAround
Copy link
Member

I will look into this once I am back next week. But this definitely needs more cleanup as we discussed (documentation, missing functions to be added etc.)

@HereAround
Copy link
Member

Unless somebody beats me to it, that is.

@HereAround HereAround force-pushed the abstract_blowups branch 6 times, most recently from 659afa8 to 49265ed Compare October 5, 2023 21:01
@HereAround HereAround force-pushed the abstract_blowups branch 6 times, most recently from f95b517 to fda89cd Compare October 6, 2023 12:16
@HereAround
Copy link
Member

As per usual, the tests may show something unexpected. That put aside, I have completed the improvements that I wanted to complete. Those changes include:

  • Code improvements (no need to remember new_ray in the type ToricBlowDownMorphism, only a single internal constructor for ToricBlowdownMorphism is needed, ...)
  • Code documentation (2 new pages for the toric varieties, one concerning all of the blowup technology and one other about the ideal sheaves - both are marked as experimental)
  • Organized the experimental code to align it with the code structure of the toric varieties (so eventually, we merely have to copy it to /src)
  • Optimized the tests (some tests were executed multiple times/created the same object repeatedly, grouped them according to subject).

Our current implementation of ideal sheaves only works for smooth toric varieties. (Reason: No time yet to look beyond it.) The previous implementation attempted to set the center of the blowup, and this failed for non-smooth examples appearing in the FTheoryTools. Since I cannot implement ideal sheaves for non-smooth toric varieties right now, I have altered this such that the blowdown morphism can be computed, but its attributes center and exceptional_divisor cannot be computed in such non-smooth settings. We (or I) will have to come back to this eventually.

Other than this... should be good to go once the tests go green.

Thank you for setting up the prototype @HechtiDerLachs !

@HereAround HereAround removed the WIP NOT ready for merging label Oct 8, 2023
@HereAround
Copy link
Member

HereAround commented Oct 8, 2023

Based on conversation with @HechtiDerLachs, this seems ready for review.

cc @afkafkafk13 @lkastner @simonbrandhorst

@HereAround HereAround marked this pull request as ready for review October 8, 2023 19:36
@HereAround
Copy link
Member

Since I hear no objections (yet) and this PR concerns experimental code, I will merge now.

Thank you @HechtiDerLachs !

@HereAround HereAround merged commit c40aeac into oscar-system:master Oct 9, 2023
8 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants