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

Pullback of rational functions along strict transforms #2681

Conversation

HechtiDerLachs
Copy link
Collaborator

Needed for the fibration hopping in the Oscar book chapter.

@afkafkafk13 : There is one important change in here: I derived BlowupMorphism from AbsCoveredSchemeMorphism. I know we talked about this some months ago and you had reasons to decide against this. But I didn't remember these reasons and, at least for me, within the last weeks it became more and more clear that it should really be like this.

Let me know if you still have objections against this.

@HechtiDerLachs
Copy link
Collaborator Author

I introduced a type for lazy composite maps of covered schemes. These should be useful in many places and I needed them to facilitate the moving around of elements of the function fields.

@codecov
Copy link

codecov bot commented Aug 16, 2023

Codecov Report

Merging #2681 (abb4910) into master (e2dfa57) will decrease coverage by 0.15%.
Report is 6 commits behind head on master.
The diff coverage is 64.65%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2681      +/-   ##
==========================================
- Coverage   72.46%   72.31%   -0.15%     
==========================================
  Files         441      441              
  Lines       62009    62494     +485     
==========================================
+ Hits        44933    45191     +258     
- Misses      17076    17303     +227     
Files Changed Coverage Δ
experimental/Schemes/Auxiliary.jl 93.15% <0.00%> (-1.00%) ⬇️
experimental/Schemes/CoveredScheme.jl 85.99% <38.18%> (-13.02%) ⬇️
experimental/Schemes/BlowupMorphism.jl 86.94% <88.88%> (+29.06%) ⬆️
experimental/Schemes/FunctionFields.jl 74.41% <100.00%> (+2.06%) ⬆️
experimental/Schemes/elliptic_surface.jl 82.03% <100.00%> (ø)

... and 36 files with indirect coverage changes

@HechtiDerLachs HechtiDerLachs mentioned this pull request Aug 17, 2023
# complements in `U_sub` and `V_sub` also is.
k = findfirst(x->has_ancestor(y->y===U_amb, codomain(inc_dom_cov[x])), patches(domain(inc_dom_cov)))
U_sub = patches(domain(inc_dom_cov))[k]
iso_U_sub = _flatten_open_subscheme(U_sub, domain(inc_dom_cov[U_sub]))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what does this do and why do you do it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line 192 is deprecated and superfluous. Did you mean that? In that case: Thanks for catching!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep

# to do the following.
# - Find a `patch` `U_sub` of the `domain` of `inc_dom_cov` for which
# inc_cod : U_sub -> W has a codomain `W` which has `U_amb` as an
# ancestor. Since `U_amb` is one of the `affine_charts` of `X`, this will work.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is U_amb a chart of X? It seems to me that it is a chart of Y?
What is the domain and codomain of p_iso? (i.e. does it point in the same direction as p? or the opposite one?)

experimental/Schemes/CoveredScheme.jl Show resolved Hide resolved
Comment on lines 691 to 697
map_string = "$(domain(f)) -> "
for i in 2:length(maps(f))
map_string = map_string * "$(domain(map(f)[i])) -> "
end
map_string = map_string * "$(codomain(map(f)[end]))"

print(io, "Composition of ", map_string)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to print to an intermediate string

Suggested change
map_string = "$(domain(f)) -> "
for i in 2:length(maps(f))
map_string = map_string * "$(domain(map(f)[i])) -> "
end
map_string = map_string * "$(codomain(map(f)[end]))"
print(io, "Composition of ", map_string)
print(io, "Composition of ", map_string)
print(io, domain(f), " -> ")
for i in 2:length(maps(f))
print(io, domain(map(f)[i]), " -> ")
end
print(io, codomain(map(f)[end]))

Comment on lines 706 to 707
end
end
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
end
end
end
print(io, Dedent())
end

experimental/Schemes/FunctionFields.jl Outdated Show resolved Hide resolved
@simonbrandhorst simonbrandhorst enabled auto-merge (squash) August 21, 2023 17:45
auto-merge was automatically disabled August 21, 2023 19:41

Pull request was closed

@simonbrandhorst simonbrandhorst enabled auto-merge (squash) August 21, 2023 19:42
@simonbrandhorst
Copy link
Collaborator

Let's pray the CI completes this time.

@simonbrandhorst simonbrandhorst merged commit 4772781 into oscar-system:master Aug 21, 2023
16 of 25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants