Skip to content

Commit

Permalink
switch to a better Julia to GAP conversion (#4086)
Browse files Browse the repository at this point in the history
* next iteration

- install `GapObj` and `GAP.julia_to_gap` methods via `GAP.@install`
- change the `GAP.julia_to_gap` calls to `GapObj` calls
- move some conversion methods from Oscar.jl to GAP.jl

* do not call `GapObj(Union{}[])`

* require GAP.jl 0.12
  • Loading branch information
ThomasBreuer authored Sep 27, 2024
1 parent 615af44 commit 32a38b3
Show file tree
Hide file tree
Showing 15 changed files with 49 additions and 86 deletions.
2 changes: 1 addition & 1 deletion Project.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ cohomCalg_jll = "5558cf25-a90e-53b0-b813-cadaa3ae7ade"
AbstractAlgebra = "0.43.1"
AlgebraicSolving = "0.7.0"
Distributed = "1.6"
GAP = "0.11.3"
GAP = "0.12.0"
Hecke = "0.34.3"
JSON = "^0.20, ^0.21"
JSON3 = "1.13.2"
Expand Down
2 changes: 1 addition & 1 deletion examples/CatSLP.jl
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ function tolazy(x, dict=IdDict{Any,Lazy}())
arguments = GenesisOfCellArguments(x)
argumentslazy = (tolazy(arguments[i], dict) for i in 1:length(arguments))
call(argumentslazy...) do args...
operation(map(arg -> arg isa Vector ? julia_to_gap(arg) : arg, args)...)
operation(map(arg -> arg isa Vector ? GapObj(arg) : arg, args)...)
end
else
Lazy(x)
Expand Down
4 changes: 2 additions & 2 deletions gap/OscarInterface/gap/alnuth.gi
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,12 @@ BindGlobal("ExponentsOfUnitsDescriptionWithRankOscar", function(F, elms)
units := JuliaToGAP(IsList, basis, true);

# the order of the torsion part of the full unit group
rank := Oscar.GAP.julia_to_gap(Oscar.order(U_m[1][1]));
rank := Oscar.GAP.GapObj(Oscar.order(U_m[1][1]));

expns := [];
for x in elms do
# map GAP int vector to Oscar element
Add(expns, Oscar.GAP.julia_to_gap(Oscar.preimage(m, K(GAPToJulia(x))).coeff)[1]);
Add(expns, Oscar.GAP.GapObj(Oscar.preimage(m, K(GAPToJulia(x))).coeff)[1]);
od;

# return result
Expand Down
4 changes: 2 additions & 2 deletions src/GAP/iso_oscar_gap.jl
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ function _make_prime_field_functions(FO, FG)
e = GAPWrap.One(FG)

f = function(x)
y = GAP.julia_to_gap(_ffe_to_int(x))::GapInt
y = GapObj(_ffe_to_int(x))::GapInt
return y*e
end

Expand Down Expand Up @@ -391,7 +391,7 @@ end

# Assume that `FO` is a `QQAbField` and `FG` is `GAP.Globals.Cyclotomics`.
function _iso_oscar_gap_abelian_closure_functions(FO::QQAbField, FG::GapObj)
return (GAP.julia_to_gap, QQAbFieldElem)
return (GapObj, QQAbFieldElem)
end

function _iso_oscar_gap(FO::QQAbField)
Expand Down
63 changes: 12 additions & 51 deletions src/GAP/oscar_to_gap.jl
Original file line number Diff line number Diff line change
Expand Up @@ -3,86 +3,47 @@
## where low level Julia objects are treated)

## `ZZRingElem` to GAP integer
function GAP.julia_to_gap(obj::ZZRingElem)
Nemo._fmpz_is_small(obj) && return GAP.julia_to_gap(Int(obj))
GAP.@install function GapObj(obj::ZZRingElem)
Nemo._fmpz_is_small(obj) && return GapObj(Int(obj))
GC.@preserve obj begin
x = Nemo._as_bigint(obj)
return ccall((:MakeObjInt, GAP.libgap), GapObj, (Ptr{UInt64}, Cint), x.d, x.size)
end
end

## `QQFieldElem` to GAP rational
GAP.julia_to_gap(obj::QQFieldElem) = GAP.Globals.QUO(GAP.julia_to_gap(numerator(obj)), GAP.julia_to_gap(denominator(obj)))
GAP.@install GapObj(obj::QQFieldElem) = GAPWrap.QUO(GapObj(numerator(obj)), GapObj(denominator(obj)))

## `PosInf` to GAP infinity
GAP.julia_to_gap(obj::PosInf) = GAP.Globals.infinity
GAP.@install GapObj(obj::PosInf) = GAP.Globals.infinity

## `ZZMatrix` to matrix of GAP integers
GAP.julia_to_gap(obj::ZZMatrix) = GAP.julia_to_gap(Matrix(obj); recursive = true)
GAP.@install GapObj(obj::ZZMatrix) = GAP.GapObj(Matrix(obj); recursive = true)

## `QQMatrix` to matrix of GAP rationals or integers
GAP.julia_to_gap(obj::QQMatrix) = GAP.julia_to_gap(Matrix(obj); recursive = true)
GAP.@install GapObj(obj::QQMatrix) = GAP.GapObj(Matrix(obj); recursive = true)

## element of cyclotomic field to GAP cyclotomic
function GAP.julia_to_gap(obj::AbsSimpleNumFieldElem)
GAP.@install function GapObj(obj::AbsSimpleNumFieldElem)
F = parent(obj)
@req Nemo.is_cyclo_type(F) "the element does not lie in a cyclotomic field"
N = get_attribute(F, :cyclo)
v = zeros(QQFieldElem, N)
coeffs = coefficients(obj)
v[1:length(coeffs)] = coeffs
return GAPWrap.CycList(GAP.julia_to_gap(v; recursive = true))
return GAPWrap.CycList(GapObj(v; recursive = true))
end

## `QQAbFieldElem` to GAP cyclotomic
function GAP.julia_to_gap(elm::QQAbFieldElem)
GAP.@install function GapObj(elm::QQAbFieldElem)
coeffs = [Nemo.coeff(elm.data, i) for i in 0:(elm.c-1)] # QQFieldElem
return GAPWrap.CycList(GapObj(coeffs; recursive = true))
end

## matrix of elements of cyclotomic field to GAP matrix of cyclotomics
function GAP.julia_to_gap(obj::AbstractAlgebra.Generic.MatSpaceElem{AbsSimpleNumFieldElem})
GAP.@install function GapObj(obj::AbstractAlgebra.Generic.MatSpaceElem{AbsSimpleNumFieldElem})
F = base_ring(obj)
@req Nemo.is_cyclo_type(F) "the matrix entries do not lie in a cyclotomic field"
mat = [GAP.julia_to_gap(obj[i,j]) for i in 1:nrows(obj), j in 1:ncols(obj)]
return GAP.julia_to_gap(mat)
end

## TODO: remove the following once GAP.jl has it
function GAP.julia_to_gap(
obj::Set{T},
recursion_dict::IdDict{Any,Any} = IdDict();
recursive::Bool = false,
) where {T}

gapset = GAP.NewPlist(length(obj))
if recursive
recursion_dict[obj] = gapset
end
for x in obj
if recursive
x = get!(recursion_dict, x) do
GAP.julia_to_gap(x, recursion_dict; recursive)
end
end
GAP.Globals.Add(gapset, x)
end
GAP.Globals.Sort(gapset)
@assert GAPWrap.IsSet(gapset)

return gapset
end

## TODO: remove the following once GAP.jl has it
## (This will be the case when the change from
## https://github.com/oscar-system/GAP.jl/pull/989
## will be available.)
using JSON3

function GAP.julia_to_gap(
obj::JSON3.Array,
recursion_dict::IdDict{Any,Any} = IdDict();
recursive::Bool = false)

return GAP.julia_to_gap(copy(obj), recursion_dict; recursive)
mat = [GapObj(obj[i,j]) for i in 1:nrows(obj), j in 1:ncols(obj)]
return GapObj(mat)
end
1 change: 1 addition & 0 deletions src/GAP/wrappers.jl
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,7 @@ GAP.@wrap PrimePGroup(x::GapObj)::GapInt
GAP.@wrap PrimitiveElement(x::GapObj)::GapObj
GAP.@wrap Projection(x::GapObj)::GapObj
GAP.@wrap Projection(x::GapObj, i::Int)::GapObj
GAP.@wrap QUO(x::GAP.Obj, y::GAP.Obj)::GAP.Obj
GAP.@wrap Random(x::GapObj, y::GapObj)::GAP.Obj
GAP.@wrap Range(x::GapObj)::GapObj
GAP.@wrap RecognizeGroup(x::GapObj)::GapObj
Expand Down
2 changes: 1 addition & 1 deletion src/Groups/GAPGroups.jl
Original file line number Diff line number Diff line change
Expand Up @@ -695,7 +695,7 @@ end
end
end

GAP.julia_to_gap(obj::GAPGroupConjClass) = obj.CC
GAP.@install GapObj(obj::GAPGroupConjClass) = obj.CC

Base.eltype(::Type{GAPGroupConjClass{T,S}}) where {T,S} = S

Expand Down
6 changes: 3 additions & 3 deletions src/Groups/cosets.jl
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ struct GroupCoset{T<: GAPGroup, S <: GAPGroupElem}
X::GapObj # GapObj(H*repr)
end

GAP.julia_to_gap(obj::GroupCoset) = obj.X
GAP.@install GapObj(obj::GroupCoset) = obj.X

Base.hash(x::GroupCoset, h::UInt) = h # FIXME
Base.eltype(::Type{GroupCoset{T,S}}) where {T,S} = S
Expand Down Expand Up @@ -317,7 +317,7 @@ struct SubgroupTransversal{T<: GAPGroup, S<: GAPGroup, E<: GAPGroupElem} <: Abst
X::GapObj # underlying *right* transversal in GAP
end

GAP.julia_to_gap(T::SubgroupTransversal, d::IdDict{Any,Any} = IdDict(); recursive::Bool = false) = T.X
GAP.@install GapObj(T::SubgroupTransversal) = T.X

function Base.show(io::IO, ::MIME"text/plain", x::SubgroupTransversal)
side = x.side === :left ? "Left" : "Right"
Expand Down Expand Up @@ -473,7 +473,7 @@ struct GroupDoubleCoset{T <: GAPGroup, S <: GAPGroupElem}
end
end

function GAP.julia_to_gap(C::GroupDoubleCoset)
GAP.@install function GapObj(C::GroupDoubleCoset)
if !isassigned(C.X)
C.X[] = GAPWrap.DoubleCoset(GapObj(C.H), GapObj(representative(C)), GapObj(C.K))
end
Expand Down
2 changes: 1 addition & 1 deletion src/Groups/group_characters.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1899,7 +1899,7 @@ GapObj(chi::GAPGroupClassFunction) = chi.values

# The following is needed for recursive `GapObj` calls with arrays
# of class functions.
GAP.julia_to_gap(chi::GAPGroupClassFunction) = chi.values
GAP.@install GapObj(chi::GAPGroupClassFunction) = chi.values

parent(chi::GAPGroupClassFunction) = chi.table

Expand Down
4 changes: 2 additions & 2 deletions src/Groups/matrices/MatGrp.jl
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ function _ring_iso(G::MatrixGroup{T}) where T
return G.ring_iso
end

function GAP.julia_to_gap(G::MatrixGroup)
GAP.@install function GapObj(G::MatrixGroup)
if !isdefined(G, :X)
if isdefined(G, :descr)
assign_from_description(G)
Expand All @@ -233,7 +233,7 @@ function GAP.julia_to_gap(G::MatrixGroup)
return G.X
end

function GAP.julia_to_gap(x::MatrixGroupElem)
GAP.@install function GapObj(x::MatrixGroupElem)
if !isdefined(x, :X)
x.X = map_entries(_ring_iso(x.parent), x.elm)
end
Expand Down
2 changes: 1 addition & 1 deletion src/Groups/matrices/forms.jl
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ function _ring_iso(f::SesquilinearForm)
return f.ring_iso
end

function GAP.julia_to_gap(f::SesquilinearForm)
GAP.@install function GapObj(f::SesquilinearForm)
if !isdefined(f, :X)
assign_from_description(f)
end
Expand Down
6 changes: 1 addition & 5 deletions src/Groups/tom.jl
Original file line number Diff line number Diff line change
Expand Up @@ -474,11 +474,7 @@ struct GAPGroupMarksVector <: GroupMarksVector
values::GapObj
end

GapObj(chi::GAPGroupMarksVector) = chi.values

# The following is needed for recursive `GapObj` calls with arrays
# of marks vectors.
GAP.julia_to_gap(chi::GAPGroupMarksVector) = GapObj(chi)
GAP.@install GapObj(chi::GAPGroupMarksVector) = chi.values

parent(chi::GAPGroupMarksVector) = chi.table

Expand Down
6 changes: 3 additions & 3 deletions src/Groups/types.jl
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ Concrete subtypes of `GAPGroup` are `PermGroup`, `FPGroup`, `SubFPGroup`,
"""
abstract type GAPGroup <: AbstractAlgebra.Group end

## `GapGroup` to GAP group
GAP.julia_to_gap(obj::GAPGroup) = obj.X
## `GapGroup` to underlying GAP group
GAP.@install GapObj(obj::GAPGroup) = obj.X

@doc raw"""
GAPGroupElem <: AbstractAlgebra.GroupElem
Expand All @@ -29,7 +29,7 @@ i.e., if `g` is a `GAPGroupElem`, then `GapObj(g)` is the `GapObj` underlying `g
abstract type GAPGroupElem{T<:GAPGroup} <: AbstractAlgebra.GroupElem end

## `GapGroupElem` to GAP group element
GAP.julia_to_gap(obj::GAPGroupElem) = obj.X
GAP.@install GapObj(obj::GAPGroupElem) = obj.X

@doc raw"""
BasicGAPGroupElem{T<:GAPGroup} <: GAPGroupElem{T}
Expand Down
7 changes: 6 additions & 1 deletion src/Serialization/GAP.jl
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,12 @@ install_GAP_deserialization(
GapObj(nameprefix)
end
init = load_node(s, :names) do names
GapObj(names; recursive = true)
if length(names) == 0
GapObj([])
else
# problem with `Union{}[]`
GapObj(names; recursive = true)
end
end
G = GAP.Globals.FreeGroup(wfilt, GAP.Globals.infinity, prefix, init)::GapObj
else
Expand Down
24 changes: 12 additions & 12 deletions test/GAP/oscar_to_gap.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2,79 +2,79 @@
# small (GAP) integer
x = ZZRingElem(17)
val = 17
@test GAP.julia_to_gap(x) == val
@test GapObj(x) == val
@test GAP.Obj(x) == val

# large GAP integer
x = ZZRingElem(2)^65
val = GAP.evalstr("2^65")
@test GAP.julia_to_gap(x) == val
@test GapObj(x) == val
@test GAP.Obj(x) == val
end

@testset "QQFieldElem" begin
# small (GAP) integer
x = ZZRingElem(17)
val = 17
@test GAP.julia_to_gap(x) == val
@test GapObj(x) == val
@test GAP.Obj(x) == val

# large GAP integer
x = ZZRingElem(2)^65
val = GAP.evalstr("2^65")
@test GAP.julia_to_gap(x) == val
@test GapObj(x) == val
@test GAP.Obj(x) == val

# non-integer rational, small numerator and denominator
x = QQFieldElem(2, 3)
val = GAP.evalstr("2/3")
@test GAP.julia_to_gap(x) == val
@test GapObj(x) == val
@test GAP.Obj(x) == val

# non-integer rational, large numerator and denominator
x = QQFieldElem(ZZRingElem(2)^65, ZZRingElem(3)^40)
val = GAP.evalstr("2^65/3^40")
@test GAP.julia_to_gap(x) == val
@test GapObj(x) == val
@test GAP.Obj(x) == val
end

@testset "ZZMatrix" begin
# matrix of small (GAP) integers
x = Nemo.ZZ[1 2; 3 4]
val = GAP.evalstr( "[ [ 1, 2 ], [ 3, 4 ] ]" )
@test GAP.julia_to_gap(x) == val
@test GapObj(x) == val
@test GAP.Obj(x) == val

# matrix containing small and large integers
x = Nemo.ZZ[1 BigInt(2)^65; 3 4]
val = GAP.evalstr( "[ [ 1, 2^65 ], [ 3, 4 ] ]" )
@test GAP.julia_to_gap(x) == val
@test GapObj(x) == val
@test GAP.Obj(x) == val
end

@testset "QQMatrix" begin
# matrix of small (GAP) integers
x = Nemo.QQ[1 2; 3 4]
val = GAP.evalstr( "[ [ 1, 2 ], [ 3, 4 ] ]" )
@test GAP.julia_to_gap(x) == val
@test GapObj(x) == val
@test GAP.Obj(x) == val

# matrix containing small and large integers
x = Nemo.QQ[1 BigInt(2)^65; 3 4]
val = GAP.evalstr( "[ [ 1, 2^65 ], [ 3, 4 ] ]" )
@test GAP.julia_to_gap(x) == val
@test GapObj(x) == val
@test GAP.Obj(x) == val

# matrix containing non-integer rationals, small numerator and denominator
x = Nemo.QQ[QQFieldElem(1, 2) 2; 3 4]
val = GAP.evalstr( "[ [ 1/2, 2 ], [ 3, 4 ] ]" )
@test GAP.julia_to_gap(x) == val
@test GapObj(x) == val
@test GAP.Obj(x) == val

# matrix containing non-integer rationals, large numerator and denominator
x = Nemo.QQ[QQFieldElem(ZZRingElem(2)^65, ZZRingElem(3)^40) 2; 3 4]
val = GAP.evalstr( "[ [ 2^65/3^40, 2 ], [ 3, 4 ] ]" )
@test GAP.julia_to_gap(x) == val
@test GapObj(x) == val
@test GAP.Obj(x) == val
end

Expand Down

0 comments on commit 32a38b3

Please sign in to comment.