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

Introduce low-level analogs for Flint structs #1889

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

fingolfin
Copy link
Member

This is starting work towards resolving #1875.

I have a few regex with which I can copy&past C struct definitions from flint headers and quickly turn them into Julia structs. However on the long run it might be better to write a little script that you point at the Flint headers in extracts everything for us automatically, that way we can re-run it on Flint updates.

Unfortunately Hecke accesses a few member variables of various of our Flint wrappers directly (e.g. mod_n, ninv), so we need to coordinate this carefully. Perhaps by adding some getter functions for those first, then changing Hecke to use those, then we can merge this (well, once it is ready).

What this does not yet show are:

  • extend unsafe funcs like add!, set! to also accept these C types directly
  • conversely e.g. mat_entry_ptr should return Ptr{fmpz_poly_struct} instead of Ptr{ZZPolyRingElem} etc.
  • afterward we can probably merge all BlahAbsPowerSeriesRingElem into a single parametrized one, and like wise for relative ones (those are basically wrappers around Flint polynomials, and we can re-use the same set!, add! etc. methods on them)
    • admittedly we probably could so right now as well, at the cost of having some extra overhead in there (i.e. wrap polynomials, which means we carry around an extra pointer to a parent polynomial ring; but that may not be worth it)
  • ... more that I forgot for now

@@ -42,7 +44,7 @@ integer_ring() = ZZRing()

@doc zz_ring_doc
mutable struct ZZRingElem <: RingElem
d::Int
d::Flint.fmpz
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
d::Flint.fmpz
data::Flint.fmpz

consistency with the other structs

@lgoettgens
Copy link
Collaborator

lgoettgens commented Oct 10, 2024

Could you provide your regexes here? I'd like to play a bit with them around

@fingolfin
Copy link
Member Author

  • Flint struct to Julia: typedef struct\s*\{([^{}]+)\}\s*([^ ;]+); -> struct \2\1end
  • Flint plain struct member to Julia: ^ *([a-z_]+) *([a-z_]+); -> \2::\1
  • Flint pointer struct member to Julia: ^ *([a-z_]+) *\* *([a-z_]+); -> \2::Ptr{\1}
  • Flint pointer pointer struct member to Julia: ^ *([a-z_]+) *\*\s*\* *([a-z_]+); -> \2::Ptr{Ptr{\1}}

This does not cover arrays (for no deep reason other than that I only encountered one instances of those, and late, so I did it manually)

norm::flint_bitcnt_t
end

struct fmpz_poly_struct
Copy link
Member Author

Choose a reason for hiding this comment

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

Things I have no thought through yet: which of these should we do (and why):

  1. fmpz_poly_struct a struct, and ZZPolyRingElem a mutable struct with finalizer (this is what we have now)
  2. fmpz_poly_struct a mutable struct, with finalizer, and ZZPolyRingElem a struct
  3. both mutable struct (and one of them has finalizer)

I did 1 because this needed the least number of changes and this PR is just an incomplete PoC

Pro for 2 would be that several high-level Julia structs wrap the same underlying FLINT structs (e.g. fmpz_poly_struct is wrapped by ZZPolyRingElem, ZZAbsPowerSeriesRingElem and ZZRelPowerSeriesRingElem) and so they could use the same finalizer (well technically they already could do that now).

Anyway, I did not try methods 2 and 3, they might not work at all for fundamental reasons, or maybe method 1 (what this PR has now) is fundamentally flawed -- gotta find some time to experiment more. But if someone else has insights, please share them.

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 find it most intuitive if the finalized is attached to the fmpz_poly_struct as that is the most fundamental data structure that occupies foreign memory. And as you pointed out, that removes some duplication between multiple Nemo types using the same flint primitives

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants