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

Update to GAP.jl 0.11.1 resp. GAP 4.13.1 #3688

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

Conversation

fingolfin
Copy link
Member

@fingolfin fingolfin commented May 4, 2024

Draft as there is a failing test I had to disable for now. We already have a fix for that in GAP, but I still need to apply it to the GAP_lib_jll . We might also just wait for GAP 4.13.1 which is due soon. But I would like to get full CI tests run now to make sure there aren't other regressions hiding.

Resolves #2341

Copy link

codecov bot commented May 14, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.52%. Comparing base (b9f564a) to head (e12c759).
Report is 5 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3688      +/-   ##
==========================================
- Coverage   81.84%   81.52%   -0.33%     
==========================================
  Files         580      580              
  Lines       79900    79864      -36     
==========================================
- Hits        65398    65107     -291     
- Misses      14502    14757     +255     
Files Coverage Δ
src/Groups/GAPGroups.jl 93.41% <ø> (-0.37%) ⬇️
src/Groups/directproducts.jl 92.85% <ø> (ø)
src/Groups/homomorphisms.jl 91.57% <ø> (-1.52%) ⬇️

... and 34 files with indirect coverage changes

@fingolfin
Copy link
Member Author

@ThomasBreuer overall this looks pretty good; we could cherry-pick the (simple) fix in GAP that fixes the describe test (which this PR currently commented out), to make a new GAP_lib_jll.

But in our discussion earlier today, it sounded as if you were aware of another issue with the new GAP / GAP.jl which is perhaps not reflected by the tests here (something about a book example) ?

@ThomasBreuer
Copy link
Member

I meant this example:

julia> U = dihedral_group(8)
Pc group of order 8

julia> optimal_perm_rep(U)
Group homomorphism
  from pc group of order 8
  to permutation group of degree 4 and order 8
 
julia> isomorphism(PermGroup, U)
Group homomorphism
  from pc group of order 8
  to permutation group of degree 8 and order 8

where the idea was to show that optimal_perm_rep(U) yields something better than the general isomorphism(PermGroup, U) but apparently the new GAP's IsomorphismPermGroup is clever enough to compute the representation on 4 points.

@thofma

This comment was marked as outdated.

@lgoettgens

This comment was marked as outdated.

@thofma

This comment was marked as outdated.

@lgoettgens

This comment was marked as outdated.

@lgoettgens
Copy link
Member

just FYI: the increased number of invalidations is reported to come from

function AbstractAlgebra.promote_rule(::Type{S}, ::Type{T}) where {RT, RET, MST, S<:AbsLocalizedRingElem{RT, RET, MST}, T<:RingElement}
AbstractAlgebra.promote_rule(RET, T) == RET ? S : Union{}
end

@fingolfin fingolfin changed the title Update to GAP.jl 0.11.0 resp. GAP 4.13.0 Update to GAP.jl 0.11.1 resp. GAP 4.13.1 Jun 13, 2024
@fingolfin
Copy link
Member Author

sigh two different crashes...

GC error (probable corruption)
Allocations: 592882531 (Pool: 592501723; Big: 380808); GC: 131

!!! ERROR in jl_ -- ABORTING !!!

thread 0 ptr queue:
~~~~~~~~~~ ptr queue top ~~~~~~~~~~
GAP.SmallBag()
==========
GAP.SmallBag()
==========
GAP.GapObj()
==========
...

and also

character fields of Brauer characters |    4      4  0.1s

[3691] signal 11 (1): Segmentation fault
in expression starting at /home/runner/work/Oscar.jl/Oscar.jl/test/Groups/group_characters.jl:1111
MarkBag at /workspace/srcdir/gap-4.13.1/src/julia_gc.c:1070 [inlined]
MarkArrayOfBags at /workspace/srcdir/gap-4.13.1/src/bags.inc:21
BagMarkFunc at /workspace/srcdir/gap-4.13.1/src/julia_gc.c:710
gc_mark_outrefs at /cache/build/tester-amdci4-9/julialang/julia-master/src/gc.c:2952 [inlined]
gc_mark_loop_serial_ at /cache/build/tester-amdci4-9/julialang/julia-master/src/gc.c:2968
gc_mark_loop_serial at /cache/build/tester-amdci4-9/julialang/julia-master/src/gc.c:2991
gc_mark_loop at /cache/build/tester-amdci4-9/julialang/julia-master/src/gc.c:3168 [inlined]
_jl_gc_collect at /cache/build/tester-amdci4-9/julialang/julia-master/src/gc.c:3557
ijl_gc_collect at /cache/build/tester-amdci4-9/julialang/julia-master/src/gc.c:3936

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

Successfully merging this pull request may close these issues.

very slow doctest for all_character_table_names (group_characters.jl:380-395)
4 participants