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

Reduction mod p for schemes #2387

Merged
merged 14 commits into from
May 17, 2023

Conversation

HechtiDerLachs
Copy link
Collaborator

@simonbrandhorst : Have a look at the test file, please. Do you think this goes in the right direction? If yes, I will extend to CoveredSchemes.

@HechtiDerLachs HechtiDerLachs marked this pull request as draft May 12, 2023 15:04
Copy link
Collaborator

@simonbrandhorst simonbrandhorst left a comment

Choose a reason for hiding this comment

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

I think this goes in the right direction.

@codecov
Copy link

codecov bot commented May 12, 2023

Codecov Report

Merging #2387 (26e292a) into master (b37fbb8) will increase coverage by 0.11%.
The diff coverage is 97.29%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2387      +/-   ##
==========================================
+ Coverage   71.44%   71.55%   +0.11%     
==========================================
  Files         384      385       +1     
  Lines       52652    52798     +146     
==========================================
+ Hits        37615    37780     +165     
+ Misses      15037    15018      -19     
Impacted Files Coverage Δ
src/AlgebraicGeometry/Schemes/Methods.jl 0.00% <0.00%> (ø)
...ry/Schemes/ProjectiveSchemes/Objects/Attributes.jl 86.86% <ø> (ø)
...eometry/Schemes/AffineSchemes/Morphisms/Methods.jl 81.81% <100.00%> (+4.39%) ⬆️
...ometry/Schemes/AffineSchemes/Objects/Attributes.jl 86.89% <100.00%> (+0.18%) ⬆️
...cGeometry/Schemes/AffineSchemes/Objects/Methods.jl 94.30% <100.00%> (+2.44%) ⬆️
...ometry/Schemes/CoveredSchemes/Morphisms/Methods.jl 83.60% <100.00%> (+39.42%) ⬆️
...Geometry/Schemes/CoveredSchemes/Objects/Methods.jl 70.83% <100.00%> (+7.67%) ⬆️
...raicGeometry/Schemes/Covering/Morphisms/Methods.jl 100.00% <100.00%> (ø)
...ebraicGeometry/Schemes/Covering/Objects/Methods.jl 81.66% <100.00%> (+2.16%) ⬆️
src/AlgebraicGeometry/Schemes/Glueing/Methods.jl 94.04% <100.00%> (+1.86%) ⬆️
... and 3 more

... and 7 files with indirect coverage changes

@HechtiDerLachs
Copy link
Collaborator Author

This should be ready to go as a first version from my side. Let's see if the tests go through. Then I'll take care of switching off some internal tests and eventually rename everything to base_change. But you can already go through the code to complain about other stuff.

@HechtiDerLachs HechtiDerLachs marked this pull request as ready for review May 15, 2023 14:43
########################################################################
# Base change
########################################################################
struct BaseChangeGlueingData{T1, T2}
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
struct BaseChangeGlueingData{T1, T2}
struct BaseChangeGlueingData{T1, T2} # used for LazyGlueing

@simonbrandhorst
Copy link
Collaborator

Could you please also add a test when one works over a non-trivial base and restricts to a fiber, i.e.
some base like $K[x,y]$ and then some base change to $K[x]$ via $y \mapsto 15, x \mapsto x$ or similar.

@HechtiDerLachs
Copy link
Collaborator Author

HechtiDerLachs commented May 17, 2023

Could you please also add a test when one works over a non-trivial base and restricts to a fiber, i.e.
some base like

I probably could. However, our schemes are not really equipped with a backend to handle these kind of relative schemes. I don't think that translating something like K[x,y] to a coefficient ring in Singular works. And if it does, it's probably not effective.

Edit: I could try to do something with my RingFlattenings, though... Just kidding. I suggest to leave this as is.

@simonbrandhorst simonbrandhorst merged commit 6e662c2 into oscar-system:master May 17, 2023
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.

3 participants