-
Notifications
You must be signed in to change notification settings - Fork 109
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
Switch CI to Julia v1.10 #1562
Switch CI to Julia v1.10 #1562
Conversation
Allocations with the |
While on vacation I noticed that |
I haven't tested it. It would be great if you could report this issue. So far, I have concentrated on the other issues with Julia v1.10:
There is evidence for other problems, e.g., |
OK, so first evidence that something is wrong with Julia v1.10 and
julia> @time using Revise, Trixi, OrdinaryDiffEq
3.221956 seconds (3.73 M allocations: 248.451 MiB, 7.26% gc time, 8.06% compilation time)
julia> @time trixi_include(default_example())
...
13.680299 seconds (24.82 M allocations: 1.517 GiB, 4.26% gc time, 99.81% compilation time: <1% of which was recompilation)
julia> @time using Revise, Trixi, OrdinaryDiffEq
10.682994 seconds (25.34 M allocations: 5.952 GiB, 6.10% gc time, 3.66% compilation time: 10% of which was recompilation)
julia> @time trixi_include(default_example())
...
69.902309 seconds (239.64 M allocations: 6.916 GiB, 1.20% gc time, 16.05% compilation time: <1% of which was recompilation)
julia> @time trixi_include(default_example())
...
58.920417 seconds (214.77 M allocations: 5.291 GiB, 0.63% gc time) And it's not just a compilation issue as one can see, it must be a runtime issue (type instabilities again?) 😢 |
Seems like StaticArrays.jl and/or StrideArrays.jl is involved: With
@code_warntype Trixi.get_node_vars(u, equations, dg, i, j, element) MethodInstance for Trixi.get_node_vars(::StrideArraysCore.PtrArray{Float64, 4, (1, 2, 3, 4), Tuple{Static.StaticInt{1}, Static.StaticInt{3}, Static.StaticInt{3}, Int64}, NTuple{4, Nothing}, NTuple{4, Static.StaticInt{1}}}, ::LinearScalarAdvectionEquation2D{Float64}, ::DGSEM{LobattoLegendreBasis{Float64, 3, SVector{3, Float64}, Matrix{Float64}, Matrix{Float64}, Matrix{Float64}}, Trixi.LobattoLegendreMortarL2{Float64, 3, Matrix{Float64}, Matrix{Float64}}, SurfaceIntegralWeakForm{FluxLaxFriedrichs{typeof(max_abs_speed_naive)}}, VolumeIntegralWeakForm}, ::Int64, ::Int64, ::Int64)
from get_node_vars(u, equations, solver::DG, indices...) @ Trixi ~/.julia/packages/Trixi/CdZpe/src/solvers/dg.jl:465
Arguments
#self#::Core.Const(Trixi.get_node_vars)
u::StrideArraysCore.PtrArray{Float64, 4, (1, 2, 3, 4), Tuple{Static.StaticInt{1}, Static.StaticInt{3}, Static.StaticInt{3}, Int64}, NTuple{4, Nothing}, NTuple{4, Static.StaticInt{1}}}
equations::LinearScalarAdvectionEquation2D{Float64}
solver::DGSEM{LobattoLegendreBasis{Float64, 3, SVector{3, Float64}, Matrix{Float64}, Matrix{Float64}, Matrix{Float64}}, Trixi.LobattoLegendreMortarL2{Float64, 3, Matrix{Float64}, Matrix{Float64}}, SurfaceIntegralWeakForm{FluxLaxFriedrichs{typeof(max_abs_speed_naive)}}, VolumeIntegralWeakForm}
indices::Tuple{Int64, Int64, Int64}
Locals
#307::Trixi.var"#307#320"{StrideArraysCore.PtrArray{Float64, 4, (1, 2, 3, 4), Tuple{Static.StaticInt{1}, Static.StaticInt{3}, Static.StaticInt{3}, Int64}, NTuple{4, Nothing}, NTuple{4, Static.StaticInt{1}}}, Tuple{Int64, Int64, Int64}}
Body::Any
1 ─ nothing
│ %2 = Trixi.:(var"#307#320")::Core.Const(Trixi.var"#307#320")
│ %3 = Core.typeof(u)::Core.Const(StrideArraysCore.PtrArray{Float64, 4, (1, 2, 3, 4), Tuple{Static.StaticInt{1}, Static.StaticInt{3}, Static.StaticInt{3}, Int64}, NTuple{4, Nothing}, NTuple{4, Static.StaticInt{1}}})
│ %4 = Core.typeof(indices)::Core.Const(Tuple{Int64, Int64, Int64})
│ %5 = Core.apply_type(%2, %3, %4)::Core.Const(Trixi.var"#307#320"{StrideArraysCore.PtrArray{Float64, 4, (1, 2, 3, 4), Tuple{Static.StaticInt{1}, Static.StaticInt{3}, Static.StaticInt{3}, Int64}, NTuple{4, Nothing}, NTuple{4, Static.StaticInt{1}}}, Tuple{Int64, Int64, Int64}})
│ (#307 = %new(%5, u, indices))
│ %7 = #307::Trixi.var"#307#320"{StrideArraysCore.PtrArray{Float64, 4, (1, 2, 3, 4), Tuple{Static.StaticInt{1}, Static.StaticInt{3}, Static.StaticInt{3}, Int64}, NTuple{4, Nothing}, NTuple{4, Static.StaticInt{1}}}, Tuple{Int64, Int64, Int64}}
│ %8 = Trixi.nvariables(equations)::Core.Const(1)
│ %9 = Trixi.Val(%8)::Core.Const(Val{1}())
│ %10 = Trixi.ntuple(%7, %9)::Tuple{Any}
│ %11 = Trixi.SVector(%10)::Any
└── return %11
@code_native Trixi.get_node_vars(u, equations, dg, i, j, element) .text
.file "get_node_vars"
.globl julia_get_node_vars_2212 # -- Begin function julia_get_node_vars_2212
.p2align 4, 0x90
.type julia_get_node_vars_2212,@function
julia_get_node_vars_2212: # @julia_get_node_vars_2212
; ┌ @ /home/mschlott/.julia/packages/Trixi/CdZpe/src/solvers/dg.jl:465 within `get_node_vars`
# %bb.0: # %top
push rbp
mov rbp, rsp
push r15
push r14
push r13
push r12
push rbx
and rsp, -32
sub rsp, 128
vxorps xmm0, xmm0, xmm0
mov qword ptr [rsp + 96], rdi # 8-byte Spill
mov qword ptr [rsp + 88], r9 # 8-byte Spill
#APP
mov rax, qword ptr fs:[0]
#NO_APP
mov r13, rcx
lea rcx, [rsp + 32]
; │ @ /home/mschlott/.julia/packages/Trixi/CdZpe/src/solvers/dg.jl:476 within `get_node_vars`
; │┌ @ ntuple.jl:48 within `ntuple`
; ││┌ @ /home/mschlott/.julia/packages/Trixi/CdZpe/src/solvers/dg.jl:476 within `#307`
; │││┌ @ /home/mschlott/.julia/packages/StrideArraysCore/COJRJ/src/ptr_array.jl:941 within `getindex`
; ││││┌ @ /home/mschlott/.julia/packages/StrideArraysCore/COJRJ/src/ptr_array.jl:930 within `_offset_ptr`
; │││││┌ @ tuple.jl:322 within `map`
movabs r12, offset "j_-_2214"
mov edi, 1
mov rbx, r8
vmovaps ymmword ptr [rsp + 32], ymm0
mov qword ptr [rsp + 72], 0
mov qword ptr [rsp + 64], 0
mov r14, qword ptr [rax - 8]
; │││││└
; │││││ @ /home/mschlott/.julia/packages/StrideArraysCore/COJRJ/src/ptr_array.jl:929 within `_offset_ptr`
; │││││┌ @ /home/mschlott/.julia/packages/StrideArraysCore/COJRJ/src/ptr_array.jl:484 within `pointer`
mov qword ptr [rsp + 32], 16
mov rax, qword ptr [r14]
mov qword ptr [rsp + 40], rax
mov qword ptr [r14], rcx
; │││││└
; │││││ @ /home/mschlott/.julia/packages/StrideArraysCore/COJRJ/src/ptr_array.jl:930 within `_offset_ptr`
; │││││┌ @ tuple.jl:322 within `map`
vzeroupper
call r12
; ││││││ @ tuple.jl:322 within `map` @ tuple.jl:322
mov rdi, r13
; ││││││ @ tuple.jl:322 within `map`
mov r15, rax
mov qword ptr [rsp + 72], rax
; ││││││ @ tuple.jl:322 within `map` @ tuple.jl:322
call r12
; ││││││ @ tuple.jl:322 within `map` @ tuple.jl:322 @ tuple.jl:319
mov rdi, rbx
; ││││││ @ tuple.jl:322 within `map` @ tuple.jl:322
mov r13, rax
mov qword ptr [rsp + 64], rax
; ││││││ @ tuple.jl:322 within `map` @ tuple.jl:322 @ tuple.jl:319
call r12
mov rdi, qword ptr [rsp + 88] # 8-byte Reload
mov rbx, rax
mov qword ptr [rsp + 56], rax
call r12
; │││││└
; │││││ @ /home/mschlott/.julia/packages/StrideArraysCore/COJRJ/src/ptr_array.jl:933 within `_offset_ptr`
; │││││┌ @ /home/mschlott/.julia/packages/StrideArraysCore/COJRJ/src/ptr_array.jl:9 within `invpermtuple`
; ││││││┌ @ /home/mschlott/.julia/packages/StrideArraysCore/COJRJ/src/ptr_array.jl:9 within `macro expansion`
mov qword ptr [rsp], r15
mov qword ptr [rsp + 8], r13
mov qword ptr [rsp + 16], rbx
mov qword ptr [rsp + 48], rax
mov qword ptr [rsp + 24], rax
movabs rax, offset jl_f_tuple
mov rsi, rsp
xor edi, edi
mov edx, 4
call rax
mov r12, qword ptr [rsp + 96] # 8-byte Reload
; │││││└└
movabs rbx, offset ijl_gc_pool_alloc
mov esi, 1136
mov edx, 16
; │││││┌ @ /home/mschlott/.julia/packages/StrideArraysCore/COJRJ/src/ptr_array.jl:9 within `invpermtuple`
; ││││││┌ @ /home/mschlott/.julia/packages/StrideArraysCore/COJRJ/src/ptr_array.jl:9 within `macro expansion`
mov r15, rax
mov r13, qword ptr [r12 + 8]
mov qword ptr [rsp + 56], rax
; │││││└└
mov rdi, qword ptr [r14 + 16]
call rbx
mov rcx, rbx
mov rbx, rax
movabs rdi, 140223945836848
mov esi, 1136
mov edx, 16
mov qword ptr [rbx - 8], rdi
mov rax, qword ptr [r12]
mov qword ptr [rsp + 64], rbx
mov qword ptr [rbx], rax
mov rdi, qword ptr [r14 + 16]
call rcx
movabs rcx, 140214323774288
movabs rdi, 140214480650128
mov edx, 3
mov qword ptr [rax - 8], rcx
mov qword ptr [rsp], rbx
mov rbx, rsp
mov qword ptr [rsp + 8], r15
movabs r15, offset ijl_apply_generic
mov qword ptr [rax], r13
mov qword ptr [rsp + 48], rax
mov qword ptr [rsp + 16], rax
mov rsi, rbx
call r15
movabs rdi, 140214480669152
; ││││└
mov rsi, rbx
mov edx, 1
mov qword ptr [rsp + 48], rax
mov qword ptr [rsp], rax
call r15
mov qword ptr [rsp + 48], rax
; ││└└
mov qword ptr [rsp], rax
xor edi, edi
mov rsi, rbx
mov edx, 1
movabs rax, offset jl_f_tuple
call rax
movabs rdi, 140214497112368
; │└
mov rsi, rbx
mov edx, 1
mov qword ptr [rsp + 48], rax
mov qword ptr [rsp], rax
call r15
mov rcx, qword ptr [rsp + 40]
mov qword ptr [r14], rcx
lea rsp, [rbp - 40]
pop rbx
pop r12
pop r13
pop r14
pop r15
pop rbp
ret
.Lfunc_end0:
.size julia_get_node_vars_2212, .Lfunc_end0-julia_get_node_vars_2212
; └
# -- End function
.section ".note.GNU-stack","",@progbits Without
@code_warntype Trixi.get_node_vars(u, equations, dg, i, j, element) MethodInstance for Trixi.get_node_vars(::StrideArraysCore.PtrArray{Float64, 4, (1, 2, 3, 4), Tuple{Static.StaticInt{1}, Static.StaticInt{3}, Static.StaticInt{3}, Int64}, NTuple{4, Nothing}, NTuple{4, Static.StaticInt{1}}}, ::LinearScalarAdvectionEquation2D{Float64}, ::DGSEM{LobattoLegendreBasis{Float64, 3, SVector{3, Float64}, Matrix{Float64}, Matrix{Float64}, Matrix{Float64}}, Trixi.LobattoLegendreMortarL2{Float64, 3, Matrix{Float64}, Matrix{Float64}}, SurfaceIntegralWeakForm{FluxLaxFriedrichs{typeof(max_abs_speed_naive)}}, VolumeIntegralWeakForm}, ::Int64, ::Int64, ::Int64)
from get_node_vars(u, equations, solver::DG, indices...) @ Trixi ~/.julia/packages/Trixi/CdZpe/src/solvers/dg.jl:465
Arguments
#self#::Core.Const(Trixi.get_node_vars)
u::StrideArraysCore.PtrArray{Float64, 4, (1, 2, 3, 4), Tuple{Static.StaticInt{1}, Static.StaticInt{3}, Static.StaticInt{3}, Int64}, NTuple{4, Nothing}, NTuple{4, Static.StaticInt{1}}}
equations::LinearScalarAdvectionEquation2D{Float64}
solver::DGSEM{LobattoLegendreBasis{Float64, 3, SVector{3, Float64}, Matrix{Float64}, Matrix{Float64}, Matrix{Float64}}, Trixi.LobattoLegendreMortarL2{Float64, 3, Matrix{Float64}, Matrix{Float64}}, SurfaceIntegralWeakForm{FluxLaxFriedrichs{typeof(max_abs_speed_naive)}}, VolumeIntegralWeakForm}
indices::Tuple{Int64, Int64, Int64}
Locals
#307::Trixi.var"#307#320"{StrideArraysCore.PtrArray{Float64, 4, (1, 2, 3, 4), Tuple{Static.StaticInt{1}, Static.StaticInt{3}, Static.StaticInt{3}, Int64}, NTuple{4, Nothing}, NTuple{4, Static.StaticInt{1}}}, Tuple{Int64, Int64, Int64}}
Body::SVector{1, Float64}
1 ─ nothing
│ %2 = Trixi.:(var"#307#320")::Core.Const(Trixi.var"#307#320")
│ %3 = Core.typeof(u)::Core.Const(StrideArraysCore.PtrArray{Float64, 4, (1, 2, 3, 4), Tuple{Static.StaticInt{1}, Static.StaticInt{3}, Static.StaticInt{3}, Int64}, NTuple{4, Nothing}, NTuple{4, Static.StaticInt{1}}})
│ %4 = Core.typeof(indices)::Core.Const(Tuple{Int64, Int64, Int64})
│ %5 = Core.apply_type(%2, %3, %4)::Core.Const(Trixi.var"#307#320"{StrideArraysCore.PtrArray{Float64, 4, (1, 2, 3, 4), Tuple{Static.StaticInt{1}, Static.StaticInt{3}, Static.StaticInt{3}, Int64}, NTuple{4, Nothing}, NTuple{4, Static.StaticInt{1}}}, Tuple{Int64, Int64, Int64}})
│ (#307 = %new(%5, u, indices))
│ %7 = #307::Trixi.var"#307#320"{StrideArraysCore.PtrArray{Float64, 4, (1, 2, 3, 4), Tuple{Static.StaticInt{1}, Static.StaticInt{3}, Static.StaticInt{3}, Int64}, NTuple{4, Nothing}, NTuple{4, Static.StaticInt{1}}}, Tuple{Int64, Int64, Int64}}
│ %8 = Trixi.nvariables(equations)::Core.Const(1)
│ %9 = Trixi.Val(%8)::Core.Const(Val{1}())
│ %10 = Trixi.ntuple(%7, %9)::Tuple{Float64}
│ %11 = Trixi.SVector(%10)::SVector{1, Float64}
└── return %11
@code_native Trixi.get_node_vars(u, equations, dg, i, j, element) .text
.file "get_node_vars"
.globl julia_get_node_vars_6674 # -- Begin function julia_get_node_vars_6674
.p2align 4, 0x90
.type julia_get_node_vars_6674,@function
julia_get_node_vars_6674: # @julia_get_node_vars_6674
; ┌ @ /home/mschlott/.julia/packages/Trixi/CdZpe/src/solvers/dg.jl:465 within `get_node_vars`
# %bb.0: # %top
push rbp
; │ @ /home/mschlott/.julia/packages/Trixi/CdZpe/src/solvers/dg.jl:476 within `get_node_vars`
; │┌ @ ntuple.jl:48 within `ntuple`
; ││┌ @ /home/mschlott/.julia/packages/Trixi/CdZpe/src/solvers/dg.jl:476 within `#307`
; │││┌ @ /home/mschlott/.julia/packages/StrideArraysCore/COJRJ/src/ptr_array.jl:941 within `getindex`
; ││││┌ @ /home/mschlott/.julia/packages/StrideArraysCore/COJRJ/src/ptr_array.jl:933 within `_offset_ptr`
; │││││┌ @ /home/mschlott/.julia/packages/StrideArraysCore/COJRJ/src/ptr_array.jl:911 within `_offset_ptr_dense`
; ││││││┌ @ pointer.jl:282 within `+`
; │││││││┌ @ boot.jl:793 within `UInt64`
mov rdx, qword ptr [rdi]
; ││││││└└
; ││││││┌ @ /home/mschlott/.julia/packages/StrideArraysCore/COJRJ/src/ptr_array.jl:902 within `_offset_dense` @ /home/mschlott/.julia/packages/StrideArraysCore/COJRJ/src/ptr_array.jl:902 @ /home/mschlott/.julia/packages/StrideArraysCore/COJRJ/src/ptr_array.jl:901
; │││││││┌ @ promotion.jl:423 within `*` @ int.jl:88
lea rax, [r9 + 2*r9]
mov rbp, rsp
; │││││││└
; │││││││ @ /home/mschlott/.julia/packages/StrideArraysCore/COJRJ/src/ptr_array.jl:902 within `_offset_dense` @ /home/mschlott/.julia/packages/StrideArraysCore/COJRJ/src/ptr_array.jl:902
; │││││││┌ @ int.jl:87 within `+`
add rax, r8
; │││││││└
; │││││││┌ @ promotion.jl:423 within `*` @ int.jl:88
lea rax, [rax + 2*rax]
; │││││││└
; │││││││ @ /home/mschlott/.julia/packages/StrideArraysCore/COJRJ/src/ptr_array.jl:902 within `_offset_dense`
; │││││││┌ @ int.jl:87 within `+`
add rax, rcx
; ││││└└└└
; ││││┌ @ /home/mschlott/.julia/packages/StrideArraysCore/COJRJ/src/ptr_array.jl:719 within `pload`
; │││││┌ @ /home/mschlott/.julia/packages/StrideArraysCore/COJRJ/src/ptr_array.jl:719 within `macro expansion`
; ││││││┌ @ pointer.jl:119 within `unsafe_load` @ pointer.jl:119
vmovsd xmm0, qword ptr [rdx + 8*rax - 104] # xmm0 = mem[0],zero
; │└└└└└└
pop rbp
ret
.Lfunc_end0:
.size julia_get_node_vars_6674, .Lfunc_end0-julia_get_node_vars_6674
; └
# -- End function
.section ".note.GNU-stack","",@progbits
|
Yep, seems like StrideArrays.jl is required to break it. If I do something like u_array = similar(Array{Float64}, u)
u_array .= u and then call @code_warntype Trixi.get_node_vars(u_array, equations, dg, i, j, element) there are no type instabilities anymore. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1562 +/- ##
==========================================
+ Coverage 89.59% 96.08% +6.49%
==========================================
Files 450 450
Lines 36155 36168 +13
==========================================
+ Hits 32390 34749 +2359
+ Misses 3765 1419 -2346
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
As far as I understand, the remaining errors are caused due to some methods being overwritten. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to got from my side!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot! Overall, there are fewer changes than I feared.
I noticed that for some tests the difference in the reference values is quite large, showing up already at the 6-7 significant digits (i.e., well within single precision accuracy). Do you have an idea why those particular cases failed, and did you run maybe one or two of them longer and compare them manually to a v1.9 result to ensure that the simulation remains stable?
One commonality I noticed is that most tests you had to modify are either AMR or shock capturing tests. These are sometimes tricky to test, since if values are close to a cut-off threshold, even a small rounding error (as could happen with different floating optimizations) can result in a non-negligible change: A cell that is refined (or not), an element where a pure DG or a mixed DG/FV solution is used etc.
Co-authored-by: Michael Schlottke-Lakemper <michael@sloede.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I missed a few spots in my initial review. They are not directly related to v1.10, but should hopefully not require any other changes and we'd have cleaned up our CI a little 😊
Once the final convo is resolved, this is good to merge from my side
Co-authored-by: Michael Schlottke-Lakemper <michael@sloede.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you very much for tackling this! I'd like to get a second review from our CI wizard though...
@ranocha Could you please take a final look as well? |
Not everything works on ARM:
We need to update our CI settings to reflect this - can we still use older versions/architectures for some MacOS tests? |
Yes. Since this is unrelated to the purpose of this PR, we should proceed here and fix the failing tests on another PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again @DanielDoehring for tackling this! 🙏
No description provided.