trixi-framework / trixi-framework/Trixi.jl

`FluxTurbo` vs `combine_conservative_and_nonconservative_fluxes`

Open
#3,234 1 comment 0 reactions 0 assignees View on GitHub
breaking discussion
Dominant language
Julia
Stars
731
Forks
167
Avg merge
2d 18h
Merged PRs (30d)
25

Description

While working on the GPU kernel, we deliberately decided to only support the `combine_conservative_and_conservative_fluxes` option for the nonconservative terms, for two mains reasons
- it is easier to implement on the GPU
- it provides a non-negligible speedup.

Because of that, in the GPU implementation we drop the support of nonconservative fluxes.
To build the semidiscretization of nonconservative terms in the traditional Trixi way we had to define:
```
surface_flux = (flux_conservative, flux_nonconservative)
```

while the on the GPU we only accept the combined option:
```
surface_flux = flux_conservative_nonconservative_combined
```

(See for the example https://github.com/trixi-framework/Trixi.jl/blob/main/examples/p4est_3d_dgsem/elixir_mhd_alfven_wave_combined_fluxes_nonperiodic.jl).

On the other hand, we have also introduced the new interface `FluxTurbo`, which only supports nonconservative fluxes when they are called as
```
volume_flux = FluxTurbo(flux_conservative, flux_nonconservative)
```

and it will give an error trying to use

```
volume_flux = FluxTurbo(flux_combined_conservative_nonconservative)
```

Having said that, I find at the moment two main problems
1) I find quite confusing that in an elixir, when we want to use the get the best out of Trixi, we have to define
```
surface_flux = flux_conservative_nonconservative_combined
volume_flux = FluxTurbo(flux_conservative, flux_nonconservative)
```
whereas, when we do not use `FluxTurbo` we would have the best performance when
```
surface_flux = flux_conservative_nonconservative_combined
volume_flux = flux_conservative_nonconservative_combined
```

2) When a new `combined` surface flux is defined, we are forced to create a specific new boundary condition that dispatches exactly on the type of the new combined flux, since we do not provide a common interface for all of them, as we instead do for `FluxTurbo`.

I see at the moment two possible solutions:
1) provide the `combined` option an interface similar to `FluxTurbo`, such as `CombinedFluxes`
```
surface_flux = CombinedFluxes(flux_conservative, flux_nonconservative)
```
where something similar to nonspecialized conservative and nonconservative fluxes can be done, if a specialization is not provided (see https://github.com/trixi-framework/Trixi.jl/blob/main/src/equations/numerical_fluxes.jl#L661-L677).

2) Recycle the `FluxTurbo` instead of introducing `CombinedFluxes` or something similar. However, this would have a different application, as here "Turbo" in `FluxTurbo` means currently two things:
- enabling SIMD instructions (on the CPU, this meaning is lost on the GPU)
- precomputing a set of prescribed variables
and none of these two things will happen when applied in that sense to the surface fluxes.

Contributor guide

Open the contributing guide

Research direction

Start with src/equations/numerical_fluxes.jl, especially the specialization around lines 661-677, and compare the FluxTurbo usage with the combined-flux example in examples/p4est_3d_dgsem/elixir_mhd_alfven_wave_combined_fluxes_nonperiodic.jl. Trace how surface and volume fluxes dispatch, including boundary conditions, then determine which interface the project should standardize on and what existing cases must continue to work.

Written by the indexing model from the issue text.

Assessment

Tech stack
julia
Domain
backend-api-design
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Needs clarification
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.