Skip to content

Parametrize FunctionField as FunctionField{T, U}#2414

Open
alexey-orlov-math wants to merge 1 commit into
Nemocas:masterfrom
alexey-orlov-math:funfield-stricter-typing
Open

Parametrize FunctionField as FunctionField{T, U}#2414
alexey-orlov-math wants to merge 1 commit into
Nemocas:masterfrom
alexey-orlov-math:funfield-stricter-typing

Conversation

@alexey-orlov-math
Copy link
Copy Markdown

Adds a second type parameter U <: PolyRingElem{T} to Generic.FunctionField and Generic.FunctionFieldElem, mirroring RationalFunctionField{T, U}. Unlike the latter (whose U may be uni- or multivariate), here U is restricted to the univariate polynomial element type

Add a second type parameter `U <: PolyRingElem{T}` to `FunctionField` and `FunctionFieldElem`, mirroring `RationalFunctionField{T, U}`. Here `U` is the (univariate) polynomial type over the coefficient field, so the underlying poly type is carried by the type rather than recomputed via `poly_type(T)`.
@thofma thofma added optimization Performance improvements or improved testing Previously, performance: must go faster release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes labels Jun 2, 2026
@thofma
Copy link
Copy Markdown
Member

thofma commented Jun 2, 2026

If the tests pass, can we treat this PR as "nonbreaking"? We would like to use this downstream soonish. Any thoughts on that @fingolfin or @lgoettgens?

@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 2, 2026

Codecov Report

❌ Patch coverage is 92.42424% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.12%. Comparing base (2657ac7) to head (f4e3f67).

Files with missing lines Patch % Lines
src/generic/FunctionField.jl 91.93% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2414      +/-   ##
==========================================
- Coverage   88.13%   88.12%   -0.01%     
==========================================
  Files         130      130              
  Lines       32948    32943       -5     
==========================================
- Hits        29038    29031       -7     
- Misses       3910     3912       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@alexey-orlov-math
Copy link
Copy Markdown
Author

failures in Hecke CI are expected:

Got exception outside of a @test
  MethodError: no method matching base_field_type(::Type{AbstractAlgebra.Generic.FunctionField{FqFieldElem, FqPolyRingElem}})
  
  Closest candidates are:
    base_field_type(::DataType)
     @ Hecke ~/.julia/packages/Hecke/bT28F/src/Hecke.jl:572
    base_field_type(::Any)
     @ Hecke ~/.julia/packages/Hecke/bT28F/src/Hecke.jl:571
    base_field_type(::Type{AbsNonSimpleNumField})
     @ Hecke ~/.julia/packages/Hecke/bT28F/src/Map/NumField.jl:187
    ...

we need to be exact here (i have a pr, waiting for this to land and version upgrade i guess?)

@alexey-orlov-math
Copy link
Copy Markdown
Author

Hecke changes to follow
https://github.com/thofma/Hecke.jl/pull/2275/changes

@lgoettgens
Copy link
Copy Markdown
Member

If the tests pass, can we treat this PR as "nonbreaking"? We would like to use this downstream soonish. Any thoughts on that @fingolfin or @lgoettgens?

I would be fine with it, but it seems like downstream CI is unhappy...

@alexey-orlov-math
Copy link
Copy Markdown
Author

If the tests pass, can we treat this PR as "nonbreaking"? We would like to use this downstream soonish. Any thoughts on that @fingolfin or @lgoettgens?

I would be fine with it, but it seems like downstream CI is unhappy...

yes and it will be fixed by
thofma/Hecke.jl#2275

this is due to the way julia resolves "overloads" - current base_field_type is "underspecified" for function field so it cannot decide. I assume (i dunno about the process) that we would need to update version and use updated version in the same pr

@thofma thofma mentioned this pull request Jun 5, 2026
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

optimization Performance improvements or improved testing Previously, performance: must go faster release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants