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

Add many more low-level helper methods such as add!, sub!, etc., and use them systematically. #1812

Open
fingolfin opened this issue Jun 27, 2024 · 5 comments

Comments

@fingolfin
Copy link
Member

Right now we ccall many FLINT functions from multiple places, e.g. fmpq_mpoly_add is called in three places in Nemo; so is fmpz_add but that one also has two callers in Hecke.

At the same time, we are not using a bunch of FLINT accessor at all, e.g. while we also wrap fmpq_mpoly_add_si we don't wrap fmpq_mpoly_add_ui (perhaps it didn't exist when these wrappers were originally written).

To address this, I suggest we make provide these low-level helpers in a more systematic way (see e.g. PR #1809), and also use them more systematically (see e.g. PR #1811). In the example above, each of these functions should ideally be ccall'ed from exactly one spot, namely an add! method. Those add! methods then can take care of dispatching to the right ccalls. The result is code that is easier to read, and often it enables us to reduce code duplication. E.g. with suitable add! methods in place, we can have this:

function +(x::QQMatrix, y::QQMatrix)
  check_parent(x, y)
  z = similar(x)
  add!(z, x, y)
end

and this

function +(x::ZZMatrix, y::ZZMatrix)
  check_parent(x, y)
  z = similar(x)
  add!(z, x, y)
end

So if we wanted to we could implement + for all Nemo matrix types in a single place. Maybe a more plausible example is how in PR #1810 several constructors for fqPolyRepField become essentially identical and could be merged.

Using macros to generate all sensible variants of some low-level wrappers as done in e.g. PR #1809 probably should be done in more place. I.e. we should do similar things for the other FLINT types we wrap. For starters let's focus on set/add/sub/mul/div; but on the long run many more could and should be added -- e.g. neg!, zero!, divexact! etc. etc.

Basically any fmpz_mat_* API in FLINT could be wrapped in such a low-level wrapper. Someone could perhaps even write a tool to generate the relevant helpers (or at least some of them) from FLINT headers? I think some language wrappers for FLINT are already handled that way; was it the Haskell one? Maybe more? Surely @albinahlback and @fredrik-johansson can clarify that.

A key for this is to have, say, add! methods which don't just accept, say, a ZZRingElem but also a Ptr{ZZRingElem} or Ref{ZZRingElem} -- this can be achieved with a union type, see also e.g. PR #1808. That way we can use these low-level helpers in many more places.

@albinahlback
Copy link
Contributor

I will ping @edgarcosta as he has worked on some parsing of headers for documentation purposes.

@albinahlback
Copy link
Contributor

Do you suggest implementing some sort of script which will automate the process of checking:

  • Which methods are implemented, and
  • Implementing those in Nemo?

@fingolfin
Copy link
Member Author

I would imagine a script that would generate the wrapper code from headers, and then we can on occasionally rerun (when FLINT_jll is updated). What exactly it generates remains to be seen. Also I don't need it to generate wrappers for everything, some selectiveness would be perfectly fine and perhaps even desirable.

(Even better would be if also tests could be generated, at least some basic ones; say some trivial ones which verify that e.g. add!(a,x,ZZ(1)) == add!(b,x,1) etc.)

@edgarcosta
Copy link

Here is the code that I wrote to parse the headers and the documentation (it is not super polished, and I am afraid that I overcomplicated it):
https://gist.github.com/edgarcosta/888fcf83565871afd635f641341cc14e

@lgoettgens
Copy link
Collaborator

lgoettgens commented Jul 16, 2024

In the example above, each of these functions should ideally be ccall'ed from exactly one spot, namely an add! method. Those add! methods then can take care of dispatching to the right calls.

One can check which FLINT functions get called from the most places using

{grep -orh "ccall((:\([a-zA-Z0-9_-]*\)" | cut -c 9- ; grep -orh "@ccall libflint.\([a-zA-Z0-9_-]*\)" | cut -c 17-;} | sort | uniq -c | sort -n

On the current master (dd58b62) the most extreme cases are

     10 acb_neg
     10 fmpq_poly_get_coeff_fmpq
     10 fmpz_mod_poly_factor_get_fmpz_mod_poly
     10 fmpz_poly_set_coeff_fmpz
     10 fmpz_poly_shift_left
     10 fq_default_poly_shift_left
     11 fmpq_poly_set_trunc
     11 fmpz_poly_set_trunc
     11 fq_default_poly_set_trunc
     12 acb_set_round
     12 arb_zero
     12 mag_zero
     12 nmod_poly_init
     13 acb_set
     13 fmpq_poly_add_series
     13 fmpz_poly_add_series
     13 fq_default_poly_add_series
     14 arb_mat_entry_ptr
     14 arb_set
     14 fmpz_mod_poly_init
     15 fmpz_mod_poly_set_coeff_fmpz
     16 arb_rad_ptr
     16 fmpz_mod_poly_init2
     17 fmpz_poly_get_coeff_fmpz
     18 arb_mid_ptr
     20 acb_real_ptr
     21 fmpq_poly_shift_left
     22 acb_imag_ptr
     23 fmpz_fdiv_ui
     24 nmod_poly_init2
     28 nmod_poly_set_coeff_ui
     32 acb_mat_entry_ptr

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

4 participants