Skip to content

Speed up computations for EllipticSurface by reduction to positive characteristic.#4615

Merged
simonbrandhorst merged 12 commits intomasterfrom
sb/good_reduction
Feb 21, 2025
Merged

Speed up computations for EllipticSurface by reduction to positive characteristic.#4615
simonbrandhorst merged 12 commits intomasterfrom
sb/good_reduction

Conversation

@simonbrandhorst
Copy link
Copy Markdown
Collaborator

@simonbrandhorst simonbrandhorst commented Feb 20, 2025

No description provided.

joschmitt
joschmitt reviewed Feb 20, 2025
View reviewed changes
Comment thread src/AlgebraicGeometry/Surfaces/EllipticSurface/Morphisms.jl
@HechtiDerLachs
Copy link
Copy Markdown
Collaborator

HechtiDerLachs commented Feb 20, 2025

has_is_isomorphism seems to be another instance of is_known, once we have it. Since #4507 is done and just waiting for merge, can we rebase on that?

On top of that: Why export all these functions now? Since recently we have looong discussions about maintaining code and stability with hours spent on whether or not we have to document and or deprecate newly introduced or dropped kwargs. In view of these difficult discussions, I'm afraid that the changes in this PR might be too premature.

@simonbrandhorst
Copy link
Copy Markdown
Collaborator Author

simonbrandhorst commented Feb 20, 2025

O.k. I will wait for is_known to be merged.
Do we promise stability of an algorithm keyword argument? Otherwise I think the interface is quite stable.

@HechtiDerLachs HechtiDerLachs mentioned this pull request Feb 20, 2025
Closed
@fingolfin
Copy link
Copy Markdown
Member

fingolfin commented Feb 20, 2025 •
edited
Loading

I've rebased this on latest master, then rebased PR #4617 by @HechtiDerLachs on this, and cleaned up several of his commits. So there are now three commits added on top by @HechtiDerLachs . One changes things to use is_known, the other two are possibly unrelated??

@fingolfin fingolfin closed this Feb 20, 2025
@fingolfin fingolfin reopened this Feb 20, 2025
fingolfin
fingolfin reviewed Feb 20, 2025
View reviewed changes
Comment thread src/Rings/mpolyquo-localizations.jl
fingolfin
fingolfin reviewed Feb 20, 2025
View reviewed changes
Comment thread src/AlgebraicGeometry/Surfaces/EllipticSurface/NeighborStep.jl Outdated
simonbrandhorst
simonbrandhorst commented Feb 20, 2025
View reviewed changes
Comment thread src/exports.jl Outdated
@simonbrandhorst
Copy link
Copy Markdown
Collaborator Author

simonbrandhorst commented Feb 20, 2025

Good to go from my side.

HechtiDerLachs
HechtiDerLachs approved these changes Feb 21, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

@HechtiDerLachs HechtiDerLachs left a comment

Choose a reason for hiding this comment

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

I reverted the commits which were not ripe yet. The rest looks good.

@simonbrandhorst
Copy link
Copy Markdown
Collaborator Author

simonbrandhorst commented Feb 21, 2025

Thanks @HechtiDerLachs

@simonbrandhorst simonbrandhorst merged commit 8881658 into master Feb 21, 2025
@simonbrandhorst simonbrandhorst deleted the sb/good_reduction branch February 21, 2025 13:56
@fingolfin fingolfin added optimization Simpler/more performant code or more/better tests topic: algebraic geometry release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes labels Feb 27, 2025
fieker pushed a commit that referenced this pull request Mar 17, 2025
@simonbrandhorst @HechtiDerLachs @fieker
Speed up computations for EllipticSurface by reduction to positive ch…
fc0de98 
…aracteristic. (#4615)

* enable use good reduction for pushforward_on_algebraic_lattice

* pass on is_isomorphism in composition

* enable use good reduction for pushforward_on_algebraic_lattice

---------

Co-authored-by: HechtiDerLachs <zach@mathematik.uni-kl.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fingolfin fingolfin fingolfin left review comments

@joschmitt joschmitt joschmitt left review comments

@HechtiDerLachs HechtiDerLachs HechtiDerLachs approved these changes

Assignees

No one assigned

Labels

optimization Simpler/more performant code or more/better tests release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes topic: algebraic geometry

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@simonbrandhorst @HechtiDerLachs @fingolfin @joschmitt