[MHD] Add logger for iMHD CFL

[y-lapeyre]

No description provided.

[github-actions]

Thanks @y-lapeyre for opening this PR!

You can do multiple things directly here:
1 - Comment pre-commit.ci run to run pre-commit checks.
2 - Comment pre-commit.ci autofix to apply fixes.
3 - Add label autofix.ci to fix authorship & pre-commit for every commit made.
4 - Add label light-ci to only trigger a reduced & faster version of the CI (need the full one before merge).
5 - Add label trigger-ci to create an empty commit to trigger the CI.

Once the workflow completes a message will appear displaying informations related to the run.

Also the PR gets automatically reviewed by gemini, you can:
1 - Comment /gemini review to trigger a review
2 - Comment /gemini summary for a summary
3 - Tag it using @gemini-code-assist either in the PR or in review comments on files

[gemini-code-assist]

Code Review

This pull request introduces a separate CFL timestep calculation for Ideal Magnetohydrodynamics (IMHD), cfl_dt_imhd, separating it from the standard hydrodynamics CFL calculation. The review feedback focuses on performance optimizations to avoid unnecessary GPU memory allocation, buffer retrievals, GPU reductions, and MPI allreduce operations when IMHD is not active (i.e., when has_psi_field is false). Additionally, initializing the field with infinity instead of a hardcoded value of 100 is recommended to prevent arbitrary timestep limits.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

I am having trouble creating individual review comments. Click here to see my feedback.

src/shammodels/sph/src/Solver.cpp (2519-2521)

Performance Optimization

To avoid allocating GPU memory for cfl_dt_imhd when has_psi_field is false, we should wrap it in an std::optional and only initialize it if has_psi_field is true. Additionally, initializing it to shambase::get_infty<Tscal>() is safer for minimum-reduction operations than a hardcoded 100 to prevent any arbitrary limits on the timestep.

            ComputeField<Tscal> cfl_dt = utility.make_compute_field<Tscal>("cfl_dt", 1, Tscal(100));
            std::optional<ComputeField<Tscal>> cfl_dt_imhd;
            if (has_psi_field) {
                cfl_dt_imhd = utility.make_compute_field<Tscal>("cfl_dt_imhd", 1, shambase::get_infty<Tscal>());
            }

src/shammodels/sph/src/Solver.cpp (2540-2542)

Performance Optimization

Remove the unconditional buffer retrieval and write access for cfl_dt_imhd from the main loop, as these have been moved inside the if (has_psi_field) block.

src/shammodels/sph/src/Solver.cpp (2585-2589)

Correctness & Performance Optimization

Remove the unconditional event completion for cfl_dt_imhd_buf from the outer scope, as it is now correctly completed with the inner kernel's event e_imhd inside the if (has_psi_field) block.

                buf_axyz.complete_event_state(e);
                vsig_buf.complete_event_state(e);
                cfl_dt_buf.complete_event_state(e);
            });

src/shammodels/sph/src/Solver.cpp (2591-2592)

Performance Optimization

Avoid performing the GPU reduction compute_rank_min() on cfl_dt_imhd when has_psi_field is false.

            Tscal rank_dt      = cfl_dt.compute_rank_min();
            Tscal imhd_rank_dt = has_psi_field ? cfl_dt_imhd->compute_rank_min() : shambase::get_infty<Tscal>();

src/shammodels/sph/src/Solver.cpp (2656-2657)

Performance Optimization

Avoid performing the expensive MPI allreduce operation allreduce_min() on imhd_rank_dt when has_psi_field is false. This is a critical optimization for scalability at large MPI rank counts.

            Tscal hydro_cfl = shamalgs::collective::allreduce_min(rank_dt);
            Tscal imhd_cfl  = has_psi_field ? shamalgs::collective::allreduce_min(imhd_rank_dt) : shambase::get_infty<Tscal>();

[github-actions]

Workflow report

workflow report corresponding to commit 63fd18f
Commiter email is yona.lapeyre@ens-lyon.fr
GitHub page artifact URL GitHub page artifact link (can expire)

Pre-commit check report

Pre-commit check: ✅

trim trailing whitespace.................................................Passed
fix end of files.........................................................Passed
check for merge conflicts................................................Passed
check that executables have shebangs.....................................Passed
check that scripts with shebangs are executable..........................Passed
check for added large files..............................................Passed
check for case conflicts.................................................Passed
check for broken symlinks................................................Passed
check yaml...............................................................Passed
detect private key.......................................................Passed
No-tabs checker..........................................................Passed
Tabs remover.............................................................Passed
cmake-format.............................................................Passed
Validate GitHub Workflows................................................Passed
clang-format.............................................................Passed
ruff check...............................................................Passed
ruff format..............................................................Passed
Check doxygen headers....................................................Passed
Check license headers....................................................Passed
Check #pragma once.......................................................Passed
Check SYCL #include......................................................Passed
No ssh in git submodules remote..........................................Passed
No UTF-8 in files (except for authors)...................................Passed

Test pipeline can run.

Clang-tidy diff report

1083 warnings generated.
Suppressed 1084 warnings (1070 in non-user code, 13 due to line filter, 1 NOLINT).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.

Doxygen diff with main

Removed warnings : 0
New warnings : 0
Warnings count : 7951 → 7951 (0.0%)

Detailed changes :

[codecov]

Codecov Report

❌ Patch coverage is 0% with 20 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/shammodels/sph/src/Solver.cpp	0.00%	20 Missing ⚠️

📢 Thoughts on this report? Let us know!

[tdavidcl]

I'm not sure about this one as is.
It is usefull to debug the CFL individually but:

• this imposes a new MPI reduction regardless
• will print even if MHD is off
• Max is enforced to be dt = 100

Maybe i should add some kind of registry of CFL that either just do a single reduction by combining the CFL or that prints the min dt CFL by category (and then we can print the MHD one here). Speaking of that yesterday i was thinking about this exact thing that maybe we want more details on what limit the CFL

[tdavidcl]

I just made this one too #1883
This was long overdo, the CFL multiplier is really a global factor to the CFL dt so it has no buisness into modifying the computations

[tdavidcl]

superseded by #1888