Add combined nonconservative GPU volume and boundary condition kernels by MarcoArtiano · Pull Request #3065 · trixi-framework/Trixi.jl

MarcoArtiano · 2026-06-04T14:29:05Z

The callback for the divergence is broken on the GPU, because we pass the full cache on the GPU, which is not filtering to not contain not-bitstype. I just avoided having these callbacks in the tests when running on the GPU by changing the analysis_callback.

Some additional notes: the volume kernel implemented only works if the volume flux is implemented following the combine_conservative_and_nonconservative_fluxes. The boundary conditions instead are for the moment only for the original implementation of the nonconservative terms. To summarize, nonconservative systems on GPU now runs only if the volume flux is passed as combine_conservative_and_nonconservative_fluxes and the surface flux is passed as surface_flux = (flux_conservative, flux_nonconservative).

Note that the calc_interface_flux! already provides the flexibility to run with plain original implementation and combined option for nonconservative systems on the GPU.

github-actions · 2026-06-04T14:29:20Z

…Trixi.jl into ma/combined_noncons_mpi

…ramework/Trixi.jl into ma/combined_noncons_mpi

Co-authored-by: Marco Artiano <57838732+MarcoArtiano@users.noreply.github.com>

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

…into ma/noncons_gpu

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

codecov · 2026-06-22T09:48:55Z

Codecov Report

❌ Patch coverage is 48.97959% with 25 lines in your changes missing coverage. Please review.
✅ Project coverage is 96.84%. Comparing base (c9f3657) to head (0f3e153).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
src/solvers/dgsem_p4est/dg_3d_gpu.jl	46.81%	25 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3065      +/-   ##
==========================================
- Coverage   96.88%   96.84%   -0.05%     
==========================================
  Files         647      647              
  Lines       50035    50079      +44     
==========================================
+ Hits        48475    48494      +19     
- Misses       1560     1585      +25

Flag	Coverage Δ
unittests	`96.84% <48.98%> (-0.05%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

benegee

Thanks again @MarcoArtiano !

I just have a few questions.

benegee · 2026-06-23T10:51:45Z


 callbacks = CallbackSet(summary_callback,
                        analysis_callback, alive_callback,
-                        save_solution,


Why was this dropped?

@MarcoArtiano ?

I'm sorry, that is when I was trying to debug and understand which callback was failing on GPU. I restored the callback.

benegee · 2026-06-23T11:01:11Z

+        Ja1_avg = 0.5f0 * (Ja1_node + Ja1_node_ii)
+        # compute the contravariant volume flux in the direction of the
+        # averaged contravariant vector
+        fluxtilde1_left, fluxtilde1_right = volume_flux(u_node, u_node_ii, Ja1_avg,


This (and similar lines below) is main difference compared to the combine_conservative_and_nonconservative_fluxes::False version, right?

Yes, exactly.

benegee · 2026-06-23T11:06:22Z

+        # Note the factor 0.5 necessary for the nonconservative fluxes based on
+        # the interpretation of global SBP operators coupled discontinuously via
+        # central fluxes/SATs


Does this still apply?

No, this one should be removed.

benegee · 2026-06-23T11:07:55Z

+@inline function calc_boundary_flux!(u, surface_flux_values, t, boundary_condition,
+                                     MeshT::Type{<:Union{P4estMesh{3},
+                                                         T8codeMesh{3}}},
+                                     have_nonconservative_terms::True,
+                                     combine_conservative_and_nonconservative_fluxes::True,
+                                     equations,
+                                     surface_integral, dg::DG, cache, i_index, j_index,
+                                     k_index, i_node_index, j_node_index,
+                                     direction_index,
+                                     element_index, boundary_index, node_coordinates,
+                                     contravariant_vectors)


I'm certainly missing a detail, but this function body looks identical to the one for have_nonconservative_terms::False.

This is consistent with:

Trixi.jl/src/solvers/dgsem_p4est/dg_3d.jl

Lines 612 to 649 in 099f4e1

@inline function calc_boundary_flux!(surface_flux_values, t, boundary_condition,

mesh::Union{P4estMesh{3}, T8codeMesh{3}},

have_nonconservative_terms::True,

combine_conservative_and_nonconservative_fluxes::True,

equations,

surface_integral, dg::DG, cache, i_index, j_index,

k_index, i_node_index, j_node_index,

direction_index,

element_index, boundary_index)

@unpack boundaries = cache

@unpack node_coordinates, contravariant_vectors = cache.elements

@unpack surface_flux = surface_integral

# Extract solution data from boundary container

u_inner = get_node_vars(boundaries.u, equations, dg, i_node_index, j_node_index,

boundary_index)

# Outward-pointing normal direction (not normalized)

normal_direction = get_normal_direction(direction_index, contravariant_vectors,

i_index, j_index, k_index, element_index)

# Coordinates at boundary node

x = get_node_coords(node_coordinates, equations, dg,

i_index, j_index, k_index, element_index)

# Call pointwise numerical flux functions for the conservative and nonconservative part

# in the normal direction on the boundary

flux = boundary_condition(u_inner, normal_direction, x, t,

surface_flux, equations)

# Copy flux to element storage in the correct orientation

for v in eachvariable(equations)

surface_flux_values[v, i_node_index, j_node_index,

direction_index, element_index] = flux[v]

end

return nothing

end

ranocha

Thanks! I have a few questions/comments.

ranocha · 2026-06-23T13:01:14Z

+@trixi_testset "elixir_mhd_alfven_wave_combined_fluxes_nonperiodic.jl Float32" begin
+    using Trixi
+    @test_trixi_include(joinpath(EXAMPLES_DIR, "p4est_3d_dgsem",
+                                 "elixir_mhd_alfven_wave_combined_fluxes_nonperiodic.jl"),
+                        l2=Float32[0.00021050235826592327,
+                                   0.0006558863204839041,
+                                   0.0002821364444400733,
+                                   0.000794748435433683,
+                                   0.0006839039307848098,
+                                   0.0006743445524692008,
+                                   0.000318156924452865,
+                                   0.0007885451771559438,
+                                   4.811726173404515e-5],
+                        linf=Float32[0.0012031070350810857,
+                                     0.004106999758487398,
+                                     0.001783097816025008,
+                                     0.004780625055122056,
+                                     0.005095902318184908,
+                                     0.003922455893839549,
+                                     0.002515549802432071,
+                                     0.004448527671538249,
+                                     0.00019839944646198146],
+                        RealT_for_test_tolerances=Float32,
+                        real_type=Float32)
+    # Ensure that we do not have excessive memory allocations
+    # (e.g., from type instabilities)
+    semi = ode.p # `semidiscretize` adapts the semi, so we need to obtain it from the ODE problem.
+    @test_allocations(Trixi.rhs!, semi, sol, 2_000_000)
+end


Can you please add the tests whether the types of everything are correct to this new testset, soo?

For the tests with KA as backend, we have never provided these type of tests. However, for the GPU backends type tests are present.

I'm not sure about the reason why these tests are missing and eventually we should open an issue and provide all the KA tests with type tests, similarly to the GPU tests.

Yes, that would be good 👍

Co-authored-by: Hendrik Ranocha <ranocha@users.noreply.github.com>

Co-authored-by: Marco Artiano <57838732+MarcoArtiano@users.noreply.github.com>

MarcoArtiano and others added 4 commits June 4, 2026 13:39

add combined for mpi 3D

eeee0d2

Update src/solvers/dgsem_p4est/dg_3d_parallel.jl

8d3a6a4

Update test/test_mpi_p4est_3d.jl

7cb76a3

add nonconservative volume kernel and boundary flux

42f0b4a

MarcoArtiano and others added 5 commits June 4, 2026 17:09

add tests

6e2e66a

move flux definitions outside

e0f0466

Merge branch 'ma/combined_noncons_mpi' of github.com:trixi-framework/…

e949772

…Trixi.jl into ma/combined_noncons_mpi

fix tests

639e967

Update src/solvers/dgsem_p4est/dg_3d_parallel.jl

f5e429e

MarcoArtiano added the gpu label Jun 4, 2026

MarcoArtiano and others added 7 commits June 11, 2026 19:32

create elixir

92b8a2e

git pull Merge branch 'ma/combined_noncons_mpi' of github.com:trixi-f…

3344c55

…ramework/Trixi.jl into ma/combined_noncons_mpi

Merge branch 'main' into ma/combined_noncons_mpi

6c4ee51

Update test/test_mpi_p4est_3d.jl

a4572ed

add comments and fix bc

da3b66c

Apply suggestions from code review

78e5543

Co-authored-by: Marco Artiano <57838732+MarcoArtiano@users.noreply.github.com>

Apply suggestions from code review

d78e3e4

Co-authored-by: Marco Artiano <57838732+MarcoArtiano@users.noreply.github.com>

MarcoArtiano changed the base branch from main to ma/combined_noncons_mpi June 11, 2026 19:39

MarcoArtiano and others added 2 commits June 11, 2026 21:54

add elixir and combined boundary flux

8888ee5

Merge branch 'ma/combined_noncons_mpi' into ma/noncons_gpu

e329549

MarcoArtiano commented Jun 11, 2026

View reviewed changes

Comment thread examples/p4est_3d_dgsem/elixir_mhd_alfven_wave_nonperiodic.jl Outdated

Apply suggestions from code review

6a63f24

Co-authored-by: Marco Artiano <57838732+MarcoArtiano@users.noreply.github.com>

Base automatically changed from ma/combined_noncons_mpi to main June 12, 2026 08:25

MarcoArtiano and others added 2 commits June 12, 2026 12:13

Update src/solvers/dgsem_p4est/dg_3d_gpu.jl

9742f3d

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

Merge branch 'main' into ma/noncons_gpu

6981dc0

MarcoArtiano commented Jun 13, 2026

View reviewed changes

Comment thread test/test_amdgpu_3d.jl Outdated

MarcoArtiano and others added 3 commits June 13, 2026 10:32

fix tests

c805e1f

Merge branch 'ma/noncons_gpu' of github.com:trixi-framework/Trixi.jl …

05ccb4b

…into ma/noncons_gpu

Merge branch 'main' into ma/noncons_gpu

4d0b2ac

MarcoArtiano and others added 4 commits June 13, 2026 11:25

fix type glm speed callback

f13a7e5

Merge branch 'ma/noncons_gpu' of github.com:trixi-framework/Trixi.jl …

f2200a5

…into ma/noncons_gpu

remove show statements

4c977bb

Merge branch 'main' into ma/noncons_gpu

ba7ebbc

github-actions Bot reviewed Jun 13, 2026

View reviewed changes

Comment thread src/callbacks_step/glm_speed.jl Outdated

MarcoArtiano and others added 7 commits June 13, 2026 20:31

Update src/callbacks_step/glm_speed.jl

ac11959

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

Merge branch 'main' into ma/noncons_gpu

1079063

Merge branch 'main' into ma/noncons_gpu

24d689a

remove callbacks from tests

4bea090

add analysis callback

49d5c4d

increase gpu allowed allocations

9051023

override default intergrals

89e51cf

remove false true pattern for nonconservative systems

0f3e153

MarcoArtiano marked this pull request as ready for review June 22, 2026 10:56

MarcoArtiano requested review from benegee, ranocha and vchuravy June 22, 2026 10:56

benegee reviewed Jun 23, 2026

View reviewed changes

ranocha added the performance We are greedy label Jun 23, 2026

ranocha requested changes Jun 23, 2026

View reviewed changes

Update src/callbacks_step/glm_speed.jl

80dfd81

Co-authored-by: Hendrik Ranocha <ranocha@users.noreply.github.com>

MarcoArtiano commented Jun 23, 2026

View reviewed changes

Comment thread src/solvers/dgsem_p4est/dg_3d_gpu.jl Outdated

MarcoArtiano added 2 commits June 23, 2026 19:06

remove obsolete comment

66110be

Co-authored-by: Marco Artiano <57838732+MarcoArtiano@users.noreply.github.com>

Merge branch 'main' into ma/noncons_gpu

57a1a54

MarcoArtiano mentioned this pull request Jun 23, 2026

KernelAbstractions type tests are missing #3099

Open

restore save solution callback

13e9818

	@inline function calc_boundary_flux!(surface_flux_values, t, boundary_condition,
	mesh::Union{P4estMesh{3}, T8codeMesh{3}},
	have_nonconservative_terms::True,
	combine_conservative_and_nonconservative_fluxes::True,
	equations,
	surface_integral, dg::DG, cache, i_index, j_index,
	k_index, i_node_index, j_node_index,
	direction_index,
	element_index, boundary_index)
	@unpack boundaries = cache
	@unpack node_coordinates, contravariant_vectors = cache.elements
	@unpack surface_flux = surface_integral

	# Extract solution data from boundary container
	u_inner = get_node_vars(boundaries.u, equations, dg, i_node_index, j_node_index,
	boundary_index)

	# Outward-pointing normal direction (not normalized)
	normal_direction = get_normal_direction(direction_index, contravariant_vectors,
	i_index, j_index, k_index, element_index)

	# Coordinates at boundary node
	x = get_node_coords(node_coordinates, equations, dg,
	i_index, j_index, k_index, element_index)

	# Call pointwise numerical flux functions for the conservative and nonconservative part
	# in the normal direction on the boundary
	flux = boundary_condition(u_inner, normal_direction, x, t,
	surface_flux, equations)

	# Copy flux to element storage in the correct orientation
	for v in eachvariable(equations)
	surface_flux_values[v, i_node_index, j_node_index,
	direction_index, element_index] = flux[v]
	end

	return nothing
	end

Conversation

MarcoArtiano commented Jun 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jun 4, 2026

Review checklist

Purpose and scope

Code quality

Documentation

Testing

Performance

Verification

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented Jun 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

benegee left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MarcoArtiano Jun 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ranocha left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

MarcoArtiano commented Jun 4, 2026 •

edited

Loading

codecov Bot commented Jun 22, 2026 •

edited

Loading

MarcoArtiano Jun 23, 2026 •

edited

Loading