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

Multiplication in local fields spends most of the time in multiplying ramification index with the valuation #1537

Closed
simonbrandhorst opened this issue Jun 5, 2024 · 2 comments · Fixed by #1567

Comments

@simonbrandhorst
Copy link
Collaborator

might be worth investigating at some point
@joschmitt

     ╎    ╎    ╎    ╎    ╎    ╎   118   @Hecke/src/LocalField/Elem.jl:648; mul!(c::Hecke.LocalFieldElem{QadicFieldElem, Hecke.EisensteinLocalField}, a::Hecke.LocalFieldElem{Qadi…
     ╎    ╎    ╎    ╎    ╎    ╎    118   @AbstractAlgebra/src/Fraction.jl:337; *(a::Int64, b::QQFieldElem)
     ╎    ╎    ╎    ╎    ╎    ╎     118   @Nemo/src/flint/fmpz.jl:349; divexact
     ╎    ╎    ╎    ╎    ╎    ╎    ╎ 118   @Nemo/src/flint/fmpz.jl:353; divexact(x::ZZRingElem, y::ZZRingElem; check::Bool)
   97╎    ╎    ╎    ╎    ╎    ╎    ╎  118   @Nemo/src/flint/FlintTypes.jl:47; ZZRingElem
@joschmitt
Copy link
Collaborator

This should be a bit better with Nemocas/Nemo.jl#1776 .
We could probably improve it more depending on the type of the field by providing a normalization (or whatever the name will be) keyword for valuation that already does the scaling by the ramification index; and then saves the multiplication for an unramified extension.
If we have this, we could add a type parameter like valuation(Int, ...) that computes the valuation as an Int (or throws an error if it is to big / a fraction). Currently, a lot of time in mul! is spent converting to Int, so that might save a bit for some field types.
However, I don't think we can do something useful in both cases for generic LocalField types where the valuation is computed as "valuation of norm divided by degree":

return valuation(norm(a))//degree(parent(a))

Thoughts?

@fieker
Copy link
Collaborator

fieker commented Jun 6, 2024 via email

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 a pull request may close this issue.

3 participants