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.

Sign up for GitHub

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

Jump to bottom

Use Base.min / Base.max in MPI reductions #2054

Merged
merged 11 commits into from
Sep 13, 2024
Merged

Use Base.min / Base.max in MPI reductions #2054

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
4a137bd
use Base.min/max in MPI.Allreduce
benegee Aug 22, 2024
ed2540d
add comments
benegee Aug 30, 2024
cc1147d
explain workdaround
benegee Aug 30, 2024
fb8a769
typo
benegee Aug 30, 2024
7cd8456
Apply suggestions from code review
benegee Aug 30, 2024
246f3ac
switch to macos-latest in mpi tests
benegee Aug 30, 2024
1d6e7f9
remove arch specification for macos-latest
benegee Aug 30, 2024
db209ec
readd arch, required by julia-actions/setup-julia
benegee Aug 30, 2024
e6152c7
Merge branch 'main' into bg/base_min_in_mpi_reduce
benegee Sep 5, 2024
6c659f5
back to macos-13 and x64
benegee Sep 5, 2024
ffad95a
Merge branch 'main' into bg/base_min_in_mpi_reduce
benegee Sep 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/auxiliary/math.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,11 @@ end
# when using `@fastmath`, which we also get from
# [Fortran](https://godbolt.org/z/Yrsa1js7P)
# or [C++](https://godbolt.org/z/674G7Pccv).
#
# Note however that such a custom reimplementation can cause incompatibilities with other
# packages. Currently we are affected by an issue with MPI.jl on ARM, see
# https://github.com/trixi-framework/Trixi.jl/issues/1922
# The workaround is to resort to Base.min / Base.max when using MPI reductions.
"""
Trixi.max(x, y, ...)

Expand Down
3 changes: 2 additions & 1 deletion src/callbacks_step/analysis.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,8 @@ function (analysis_callback::AnalysisCallback)(io, du, u, u_ode, t, semi)
res = maximum(abs, view(du, v, ..))
if mpi_isparallel()
# TODO: Debugging, here is a type instability
global_res = MPI.Reduce!(Ref(res), max, mpi_root(), mpi_comm())
# Base.max instead of max needed, see comment in src/auxiliary/math.jl
global_res = MPI.Reduce!(Ref(res), Base.max, mpi_root(), mpi_comm())
if mpi_isroot()
res::eltype(du) = global_res[]
end
Expand Down
3 changes: 2 additions & 1 deletion src/callbacks_step/analysis_dg2d_parallel.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,8 @@ function calc_error_norms(func, u, t, analyzer,
global_l2_error = Vector(l2_error)
global_linf_error = Vector(linf_error)
MPI.Reduce!(global_l2_error, +, mpi_root(), mpi_comm())
MPI.Reduce!(global_linf_error, max, mpi_root(), mpi_comm())
# Base.max instead of max needed, see comment in src/auxiliary/math.jl
MPI.Reduce!(global_linf_error, Base.max, mpi_root(), mpi_comm())
total_volume = MPI.Reduce(volume, +, mpi_root(), mpi_comm())
if mpi_isroot()
l2_error = convert(typeof(l2_error), global_l2_error)
Expand Down
3 changes: 2 additions & 1 deletion src/callbacks_step/analysis_dg3d_parallel.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ function calc_error_norms(func, u, t, analyzer,
global_l2_error = Vector(l2_error)
global_linf_error = Vector(linf_error)
MPI.Reduce!(global_l2_error, +, mpi_root(), mpi_comm())
MPI.Reduce!(global_linf_error, max, mpi_root(), mpi_comm())
# Base.max instead of max needed, see comment in src/auxiliary/math.jl
MPI.Reduce!(global_linf_error, Base.max, mpi_root(), mpi_comm())
total_volume = MPI.Reduce(volume, +, mpi_root(), mpi_comm())
if mpi_isroot()
l2_error = convert(typeof(l2_error), global_l2_error)
Expand Down
18 changes: 12 additions & 6 deletions src/callbacks_step/stepsize_dg2d.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@ function max_dt(u, t, mesh::ParallelTreeMesh{2},
typeof(constant_speed), typeof(equations), typeof(dg),
typeof(cache)},
u, t, mesh, constant_speed, equations, dg, cache)
dt = MPI.Allreduce!(Ref(dt), min, mpi_comm())[]
# Base.min instead of min needed, see comment in src/auxiliary/math.jl
dt = MPI.Allreduce!(Ref(dt), Base.min, mpi_comm())[]

return dt
end
Expand All @@ -70,7 +71,8 @@ function max_dt(u, t, mesh::ParallelTreeMesh{2},
typeof(constant_speed), typeof(equations), typeof(dg),
typeof(cache)},
u, t, mesh, constant_speed, equations, dg, cache)
dt = MPI.Allreduce!(Ref(dt), min, mpi_comm())[]
# Base.min instead of min needed, see comment in src/auxiliary/math.jl
dt = MPI.Allreduce!(Ref(dt), Base.min, mpi_comm())[]

return dt
end
Expand Down Expand Up @@ -154,7 +156,8 @@ function max_dt(u, t, mesh::ParallelP4estMesh{2},
typeof(constant_speed), typeof(equations), typeof(dg),
typeof(cache)},
u, t, mesh, constant_speed, equations, dg, cache)
dt = MPI.Allreduce!(Ref(dt), min, mpi_comm())[]
# Base.min instead of min needed, see comment in src/auxiliary/math.jl
dt = MPI.Allreduce!(Ref(dt), Base.min, mpi_comm())[]

return dt
end
Expand All @@ -170,7 +173,8 @@ function max_dt(u, t, mesh::ParallelP4estMesh{2},
typeof(constant_speed), typeof(equations), typeof(dg),
typeof(cache)},
u, t, mesh, constant_speed, equations, dg, cache)
dt = MPI.Allreduce!(Ref(dt), min, mpi_comm())[]
# Base.min instead of min needed, see comment in src/auxiliary/math.jl
dt = MPI.Allreduce!(Ref(dt), Base.min, mpi_comm())[]

return dt
end
Expand All @@ -186,7 +190,8 @@ function max_dt(u, t, mesh::ParallelT8codeMesh{2},
typeof(constant_speed), typeof(equations), typeof(dg),
typeof(cache)},
u, t, mesh, constant_speed, equations, dg, cache)
dt = MPI.Allreduce!(Ref(dt), min, mpi_comm())[]
# Base.min instead of min needed, see comment in src/auxiliary/math.jl
dt = MPI.Allreduce!(Ref(dt), Base.min, mpi_comm())[]

return dt
end
Expand All @@ -202,7 +207,8 @@ function max_dt(u, t, mesh::ParallelT8codeMesh{2},
typeof(constant_speed), typeof(equations), typeof(dg),
typeof(cache)},
u, t, mesh, constant_speed, equations, dg, cache)
dt = MPI.Allreduce!(Ref(dt), min, mpi_comm())[]
# Base.min instead of min needed, see comment in src/auxiliary/math.jl
dt = MPI.Allreduce!(Ref(dt), Base.min, mpi_comm())[]

return dt
end
Expand Down
12 changes: 8 additions & 4 deletions src/callbacks_step/stepsize_dg3d.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,8 @@ function max_dt(u, t, mesh::ParallelP4estMesh{3},
typeof(constant_speed), typeof(equations), typeof(dg),
typeof(cache)},
u, t, mesh, constant_speed, equations, dg, cache)
dt = MPI.Allreduce!(Ref(dt), min, mpi_comm())[]
# Base.min instead of min needed, see comment in src/auxiliary/math.jl
dt = MPI.Allreduce!(Ref(dt), Base.min, mpi_comm())[]

return dt
end
Expand All @@ -146,7 +147,8 @@ function max_dt(u, t, mesh::ParallelP4estMesh{3},
typeof(constant_speed), typeof(equations), typeof(dg),
typeof(cache)},
u, t, mesh, constant_speed, equations, dg, cache)
dt = MPI.Allreduce!(Ref(dt), min, mpi_comm())[]
# Base.min instead of min needed, see comment in src/auxiliary/math.jl
dt = MPI.Allreduce!(Ref(dt), Base.min, mpi_comm())[]

return dt
end
Expand All @@ -162,7 +164,8 @@ function max_dt(u, t, mesh::ParallelT8codeMesh{3},
typeof(constant_speed), typeof(equations), typeof(dg),
typeof(cache)},
u, t, mesh, constant_speed, equations, dg, cache)
dt = MPI.Allreduce!(Ref(dt), min, mpi_comm())[]
# Base.min instead of min needed, see comment in src/auxiliary/math.jl
dt = MPI.Allreduce!(Ref(dt), Base.min, mpi_comm())[]

return dt
end
Expand All @@ -178,7 +181,8 @@ function max_dt(u, t, mesh::ParallelT8codeMesh{3},
typeof(constant_speed), typeof(equations), typeof(dg),
typeof(cache)},
u, t, mesh, constant_speed, equations, dg, cache)
dt = MPI.Allreduce!(Ref(dt), min, mpi_comm())[]
# Base.min instead of min needed, see comment in src/auxiliary/math.jl
dt = MPI.Allreduce!(Ref(dt), Base.min, mpi_comm())[]

return dt
end
Expand Down
Loading