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

Refactor our Series types: wrap polynomials, possibly use a parametric single type (or two, for Abs vs Rel) #2017

Open
fingolfin opened this issue Feb 5, 2025 · 3 comments

Comments

@fingolfin
Copy link
Member

... to avoid a ton of code duplication (including many ccall).

Of course we have some unique functionality for some of these rings, but there should be no problem to retain that.

One "downside" is that if we wrap PolyRingElem then those all have a parent, so we waste one extra polyring, plus 8 bytes for a pointer to it in each series elem. But that seems acceptable?.

So the idea would to be have something like

@attributes mutable struct FlintAbsPowerSeriesRing{T} <: SeriesRing{T}
  prec_max::Int
  S::Symbol

  ...
end

struct FlintSeriesAbsPowerSeriesRingElem{T} <: AbsPowerSeriesRingElem{T}
  poly:T
  prec:: Int
  parent::FlintAbsPowerSeriesRing{T}
end

For backwards compatibility we also can have

const QQAbsPowerSeriesRing = FlintAbsPowerSeriesRing{QQFieldElem}
const QQAbsPowerSeriesRingElem = FlintSeriesAbsPowerSeriesRingElem{QQFieldElem}

etc.

@fingolfin
Copy link
Member Author

(Actually if done right we could probably move this generic type to AbstractAlgebra, and use it for the series implementation there)

@thofma
Copy link
Member

thofma commented Feb 5, 2025

I am not convinced that the gain (less code?) outweighs the increase in memory consumption.

I think @fieker had some ideas of consolidating the series code, but I don't remember if it meant rewriting the generic code to directly work on entries or using polynomials.

@fieker
Copy link
Contributor

fieker commented Feb 5, 2025 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

No branches or pull requests

3 participants