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

Fibration hop functionality #2715

Merged

Conversation

HechtiDerLachs
Copy link
Collaborator

I add some ported functionality for @simonbrandhorst from earlier python scripts.

experimental/Schemes/elliptic_surface.jl Outdated Show resolved Hide resolved
experimental/Schemes/elliptic_surface.jl Outdated Show resolved Hide resolved
experimental/Schemes/elliptic_surface.jl Outdated Show resolved Hide resolved
@assert F === parent(x1) "something is wrong with caching of fraction fields"
# TODO: eventually add the inverse.
trans = MapFromFunc(F, F, f->evaluate(numerator(f), [x1, y1])//evaluate(denominator(f), [x1, y1]))
return trans(F(g)), trans
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here we actually care about the factor of F(g) which is in weierstrass form.
So you would

  1. factor numerator(F(g))
  2. get the factor h of g which is in weierstrass form and discard the other factors.
  3. Return h, trans

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

How do we find the factor in Weierstrass form? I just extrapolated from our vinberg script here. Is this 100% mandatory what your write here?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I would say that we look at its monomials and make sure that they
contain [y^2, x^3] and are all contained in [y^2, x*y, y, x^3, x^2, x, 1].
Yes, I would say that it is 95% mandatory. Because otherwise we will have to fish it out by hand each time and cannot automatize anything or fish it out later in the automatization. This seems like the best place for fishing for the (long) weierstrass equation.

experimental/Schemes/elliptic_surface.jl Outdated Show resolved Hide resolved
experimental/Schemes/elliptic_surface.jl Outdated Show resolved Hide resolved
experimental/Schemes/elliptic_surface.jl Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Aug 23, 2023

Codecov Report

Merging #2715 (87953f9) into master (bcff139) will decrease coverage by 0.04%.
Report is 6 commits behind head on master.
The diff coverage is 81.59%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2715      +/-   ##
==========================================
- Coverage   72.72%   72.69%   -0.04%     
==========================================
  Files         443      444       +1     
  Lines       63107    63348     +241     
==========================================
+ Hits        45896    46049     +153     
- Misses      17211    17299      +88     
Files Changed Coverage Δ
...erimental/Schemes/MorphismFromRationalFunctions.jl 44.09% <0.00%> (-0.12%) ⬇️
src/Serialization/upgrades/0.11.3.jl 0.00% <0.00%> (-85.72%) ⬇️
src/Serialization/upgrades/0.12.0.jl 0.00% <0.00%> (-92.60%) ⬇️
src/Serialization/upgrades/0.12.2.jl 0.00% <0.00%> (-94.45%) ⬇️
src/utils/docs.jl 0.00% <0.00%> (ø)
src/Serialization/upgrades/main.jl 8.82% <6.89%> (-91.18%) ⬇️
src/Serialization/TropicalGeometry.jl 70.00% <64.00%> (-30.00%) ⬇️
src/TropicalGeometry/semiring.jl 25.00% <75.00%> (-0.97%) ⬇️
src/Serialization/basic_types.jl 58.33% <82.35%> (-38.73%) ⬇️
src/Serialization/main.jl 83.33% <86.70%> (-6.97%) ⬇️
... and 13 more

... and 15 files with indirect coverage changes

@simonbrandhorst
Copy link
Collaborator

Tests will need to be adjusted to the new names too.

@simonbrandhorst
Copy link
Collaborator

Could you please add a test for each method over a function field? Just to make sure it runs and we do not need to fix it later. Because that is where we will actually use it.

@simonbrandhorst simonbrandhorst enabled auto-merge (squash) August 23, 2023 14:36
@simonbrandhorst simonbrandhorst merged commit d27997e into oscar-system:master Aug 23, 2023
9 of 12 checks passed
@HechtiDerLachs HechtiDerLachs deleted the fibration_hop_functionality branch August 24, 2023 08:10
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