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

Improve conversion between GAP and OSCAR finite fields #4581

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fingolfin
Copy link
Member

This is a change I started last April and never finished. I am resurrecting it now after a discussion with @mjrodgers and @ThomasBreuer

I wanted to refactor and possibly optimize the conversion between GAP and OSCAR finite field elements. While I got some modest improvements in one case, another got slightly slower. I also realized that there are some serious bottlenecks in Nemo that we probably should take care of. More on that below.

For now, some (old) timiings:

Before:

julia> FO = GF(3,4); FG = GAP.Globals.GF(3,4);

julia> f, finv = Oscar._iso_oscar_gap_field_finite_functions(FO, FG);

julia> e = one(FO) ; @btime f(e);
  1.875 μs (28 allocations: 832 bytes)

julia> e = one(FG) ; @btime finv(e);
  3.594 μs (38 allocations: 1.62 KiB)

After:

julia> FO = GF(3,4); FG = GAP.Globals.GF(3,4);

julia> f, finv = Oscar._iso_oscar_gap_field_finite_functions(FO, FG);

julia> e = one(FO) ; @btime f(e);
  1.429 μs (24 allocations: 624 bytes)

julia> e = one(FG) ; @btime finv(e);
  2.991 μs (39 allocations: 1.64 KiB)

One limitation is the conversion from a list of coefficients into an Nemo FFE (finite
field element). It looks like this:

function (a::FqField)(b::Vector{<:IntegerUnion})
  da = degree(a)
  db = length(b)
  da == db || error("Coercion impossible")
  return a(parent(defining_polynomial(a))(b))
end

So we take an integer vector, convert that into a polynomial, and feed that into C code which then copies the coefficients out of the polynomial, and then we discard the polynomial.

Perhaps @mjrodgers can file an issue at Nemo.jl about this, so we don't forget to improve this at some point.

Before:

    julia> FO = GF(3,4); FG = GAP.Globals.GF(3,4);

    julia> f, finv = Oscar._iso_oscar_gap_field_finite_functions(FO, FG);

    julia> e = one(FO) ; @Btime f(e);
      1.875 μs (28 allocations: 832 bytes)

    julia> e = one(FG) ; @Btime finv(e);
      3.594 μs (38 allocations: 1.62 KiB)

After:

    julia> FO = GF(3,4); FG = GAP.Globals.GF(3,4);

    julia> f, finv = Oscar._iso_oscar_gap_field_finite_functions(FO, FG);

    julia> e = one(FO) ; @Btime f(e);
      1.429 μs (24 allocations: 624 bytes)

    julia> e = one(FG) ; @Btime finv(e);
      2.991 μs (39 allocations: 1.64 KiB)
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 this pull request may close these issues.

1 participant