From 3440748680881fabd27b6a8ff17d402ec36a3bd2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lars=20G=C3=B6ttgens?= Date: Tue, 22 Oct 2024 17:00:10 +0200 Subject: [PATCH] Some fixes and performance improvements for `iso_oscar_gap(::LieAlgebra)` (#4227) * Make `_iso_oscar_gap(::AbstractLieAlgeba)` way more efficient * Move root system iso to its own function * Some type stability * Remove some unwanted iso caching --- experimental/LieAlgebras/src/iso_gap_oscar.jl | 8 +- experimental/LieAlgebras/src/iso_oscar_gap.jl | 78 +++++++++++-------- .../LieAlgebras/test/iso_gap_oscar-test.jl | 2 - .../LieAlgebras/test/iso_oscar_gap-test.jl | 1 - src/GAP/iso_oscar_gap.jl | 4 +- src/GAP/wrappers.jl | 1 + 6 files changed, 49 insertions(+), 45 deletions(-) diff --git a/experimental/LieAlgebras/src/iso_gap_oscar.jl b/experimental/LieAlgebras/src/iso_gap_oscar.jl index 7e5e259a3b86..f011aeb72694 100644 --- a/experimental/LieAlgebras/src/iso_gap_oscar.jl +++ b/experimental/LieAlgebras/src/iso_gap_oscar.jl @@ -26,9 +26,7 @@ function _iso_gap_oscar_abstract_lie_algebra( LO = _abstract_lie_algebra_from_GAP(LG, coeffs_iso, s) finv, f = _iso_oscar_gap_lie_algebra_functions(LO, LG, inv(coeffs_iso)) - iso = MapFromFunc(LG, LO, f, finv) - set_attribute!(LO, :iso_oscar_gap => inv(iso)) - return iso + return MapFromFunc(LG, LO, f, finv) end function _iso_gap_oscar_linear_lie_algebra( @@ -39,9 +37,7 @@ function _iso_gap_oscar_linear_lie_algebra( LO = _linear_lie_algebra_from_GAP(LG, coeffs_iso, s) finv, f = _iso_oscar_gap_lie_algebra_functions(LO, LG, inv(coeffs_iso)) - iso = MapFromFunc(LG, LO, f, finv) - set_attribute!(LO, :iso_oscar_gap => inv(iso)) - return iso + return MapFromFunc(LG, LO, f, finv) end function _abstract_lie_algebra_from_GAP( diff --git a/experimental/LieAlgebras/src/iso_oscar_gap.jl b/experimental/LieAlgebras/src/iso_oscar_gap.jl index 1b8aa40ac95e..b01734970c02 100644 --- a/experimental/LieAlgebras/src/iso_oscar_gap.jl +++ b/experimental/LieAlgebras/src/iso_oscar_gap.jl @@ -22,7 +22,7 @@ function _iso_oscar_gap_lie_algebra_functions( return (f, finv) end -function _iso_oscar_gap(LO::LinearLieAlgebra) +function _iso_oscar_gap(LO::LinearLieAlgebra; set_attributes::Bool=true) coeffs_iso = Oscar.iso_oscar_gap(coefficient_ring(LO)) LG = GAP.Globals.LieAlgebra( codomain(coeffs_iso), @@ -32,6 +32,12 @@ function _iso_oscar_gap(LO::LinearLieAlgebra) f, finv = _iso_oscar_gap_lie_algebra_functions(LO, LG, coeffs_iso) + if set_attributes && has_root_system(LO) + # we need to construct the root system in GAP as otherwise it may detect a different order of simple roots + RO = root_system(LO) + _iso_oscar_gap_set_root_system(LG, RO) + end + return MapFromFunc(LO, LG, f, finv) end @@ -41,17 +47,17 @@ function _iso_oscar_gap(LO::AbstractLieAlgebra; set_attributes::Bool=true) [ [ begin - pairs = filter(pair -> !iszero(last(pair)), collect(enumerate(_matrix(xi * xj)))) - (map(first, pairs), GAP.Obj[coeffs_iso(c) for c in map(last, pairs)]) - end for xj in basis(LO) - ] for xi in basis(LO) + pairs = collect(LO.struct_consts[i, j]) + (first.(pairs), GAP.Obj[coeffs_iso(c) for c in last.(pairs)]) + end for j in 1:dim(LO) + ] for i in 1:dim(LO) ] -1 coeffs_iso(zero(coefficient_ring(LO))) ] - LG = GAP.Globals.LieAlgebraByStructureConstants( - codomain(coeffs_iso), GAP.Obj(sc_table_G; recursive=true) + LG = GAPWrap.LieAlgebraByStructureConstants( + codomain(coeffs_iso), GapObj(sc_table_G; recursive=true) ) f, finv = _iso_oscar_gap_lie_algebra_functions(LO, LG, coeffs_iso) @@ -59,34 +65,38 @@ function _iso_oscar_gap(LO::AbstractLieAlgebra; set_attributes::Bool=true) if set_attributes && has_root_system(LO) # we need to construct the root system in GAP as otherwise it may detect a different order of simple roots RO = root_system(LO) - RG = GAP.Globals.Objectify( - GAP.Globals.NewType( - GAP.Globals.NewFamily(GAP.Obj("RootSystemFam"), GAP.Globals.IsObject), - GAP.evalstr("IsAttributeStoringRep and IsRootSystemFromLieAlgebra")), - GAP.GapObj(Dict{Symbol,Any}())) - GAP.Globals.SetUnderlyingLieAlgebra(RG, LG) - - cartan_trO = transpose(cartan_matrix(RO)) - transform_root(r::RootSpaceElem) = GAP.Obj(coefficients(r) * cartan_trO)[1] - GAP.Globals.SetPositiveRoots(RG, GAP.Obj(transform_root.(positive_roots(RO)))) - GAP.Globals.SetNegativeRoots(RG, GAP.Obj(transform_root.(negative_roots(RO)))) - GAP.Globals.SetSimpleSystem(RG, GAP.Obj(transform_root.(simple_roots(RO)))) - can_basisG = GAP.Globals.CanonicalBasis(LG) - pos_root_vectorsG = can_basisG[1:n_positive_roots(RO)] - neg_root_vectorsG = can_basisG[(n_positive_roots(RO) + 1):(2 * n_positive_roots(RO))] - csa_basisG = can_basisG[(2 * n_positive_roots(RO) + 1):end] - GAP.Globals.SetPositiveRootVectors(RG, pos_root_vectorsG) - GAP.Globals.SetNegativeRootVectors(RG, neg_root_vectorsG) - GAP.Globals.SetCanonicalGenerators( - RG, GAP.Obj([pos_root_vectorsG, neg_root_vectorsG, csa_basisG]) - ) - GAP.Globals.SetChevalleyBasis( - LG, GAP.Obj([pos_root_vectorsG, neg_root_vectorsG, csa_basisG]) - ) - - GAP.Globals.SetCartanMatrix(RG, GAP.Obj(cartan_trO)) - GAP.Globals.SetRootSystem(LG, RG) + _iso_oscar_gap_set_root_system(LG, RO) end return MapFromFunc(LO, LG, f, finv) end + +function _iso_oscar_gap_set_root_system(LG::GapObj, RO::RootSystem) + RG = GAP.Globals.Objectify( + GAP.Globals.NewType( + GAP.Globals.NewFamily(GAP.Obj("RootSystemFam"), GAP.Globals.IsObject), + GAP.evalstr("IsAttributeStoringRep and IsRootSystemFromLieAlgebra")), + GAP.GapObj(Dict{Symbol,Any}())) + GAP.Globals.SetUnderlyingLieAlgebra(RG, LG) + + cartan_trO = transpose(cartan_matrix(RO)) + transform_root(r::RootSpaceElem) = GAP.Obj(coefficients(r) * cartan_trO)[1] + GAP.Globals.SetPositiveRoots(RG, GAP.Obj(transform_root.(positive_roots(RO)))) + GAP.Globals.SetNegativeRoots(RG, GAP.Obj(transform_root.(negative_roots(RO)))) + GAP.Globals.SetSimpleSystem(RG, GAP.Obj(transform_root.(simple_roots(RO)))) + can_basisG = GAP.Globals.CanonicalBasis(LG) + pos_root_vectorsG = can_basisG[1:n_positive_roots(RO)] + neg_root_vectorsG = can_basisG[(n_positive_roots(RO) + 1):(2 * n_positive_roots(RO))] + csa_basisG = can_basisG[(2 * n_positive_roots(RO) + 1):end] + GAP.Globals.SetPositiveRootVectors(RG, pos_root_vectorsG) + GAP.Globals.SetNegativeRootVectors(RG, neg_root_vectorsG) + GAP.Globals.SetCanonicalGenerators( + RG, GAP.Obj([pos_root_vectorsG, neg_root_vectorsG, csa_basisG]) + ) + GAP.Globals.SetChevalleyBasis( + LG, GAP.Obj([pos_root_vectorsG, neg_root_vectorsG, csa_basisG]) + ) + + GAP.Globals.SetCartanMatrix(RG, GAP.Obj(cartan_trO)) + GAP.Globals.SetRootSystem(LG, RG) +end diff --git a/experimental/LieAlgebras/test/iso_gap_oscar-test.jl b/experimental/LieAlgebras/test/iso_gap_oscar-test.jl index 6870438dfc5a..206d5b2d3ba9 100644 --- a/experimental/LieAlgebras/test/iso_gap_oscar-test.jl +++ b/experimental/LieAlgebras/test/iso_gap_oscar-test.jl @@ -6,8 +6,6 @@ function test_iso_gap_oscar(LG, oscarT; num_random_tests::Int=10) LO = codomain(iso) @test LO isa oscarT - @test codomain(Oscar.iso_oscar_gap(LO)) === LG - @test iso === Oscar.iso_gap_oscar(LG) # test caching for _ in 1:num_random_tests diff --git a/experimental/LieAlgebras/test/iso_oscar_gap-test.jl b/experimental/LieAlgebras/test/iso_oscar_gap-test.jl index f01c34998365..06682e11a2bf 100644 --- a/experimental/LieAlgebras/test/iso_oscar_gap-test.jl +++ b/experimental/LieAlgebras/test/iso_oscar_gap-test.jl @@ -5,7 +5,6 @@ function test_iso_oscar_gap(LO::LieAlgebra; num_random_tests::Int=10) @test domain(iso) === LO LG = codomain(iso) @test GAPWrap.IsLieAlgebra(LG) - # @test codomain(Oscar.iso_gap_oscar(LG)) === LO @test iso === Oscar.iso_oscar_gap(LO) # test caching diff --git a/src/GAP/iso_oscar_gap.jl b/src/GAP/iso_oscar_gap.jl index 6cbf343306db..7d87de40db34 100644 --- a/src/GAP/iso_oscar_gap.jl +++ b/src/GAP/iso_oscar_gap.jl @@ -402,7 +402,7 @@ function _iso_oscar_gap(FO::QQAbField) end """ - Oscar.iso_oscar_gap(R) -> Map{T, GapObj} + Oscar.iso_oscar_gap(R::T) -> Map{T, GapObj} Return an isomorphism `f` with domain `R` and `codomain` a GAP object `S`. @@ -481,7 +481,7 @@ true structure is not fully supported in GAP will likely cause overhead at runtime. """ -@attr Map function iso_oscar_gap(F) +@attr Map{T, GapObj} function iso_oscar_gap(F::T) where T return _iso_oscar_gap(F) end diff --git a/src/GAP/wrappers.jl b/src/GAP/wrappers.jl index a12f1e309d6b..5cdfbc00cd2e 100644 --- a/src/GAP/wrappers.jl +++ b/src/GAP/wrappers.jl @@ -270,6 +270,7 @@ GAP.@wrap LargestMovedPoint(x::Any)::Int GAP.@wrap LeftActingDomain(x::GapObj)::GapObj GAP.@wrap LetterRepAssocWord(x::GapObj)::GapObj GAP.@wrap LibInfoCharacterTable(x::GapObj)::GapObj +GAP.@wrap LieAlgebraByStructureConstants(x::GapObj, y::GapObj)::GapObj GAP.@wrap LinearCharacters(x::GapObj)::GapObj GAP.@wrap LinearCombination(x::GapObj, y::GapObj)::GapObj GAP.@wrap ListPerm(x::GapObj)::GapObj