[SPH] Apply CFL multiplier at the end of the CFL computation#1883
[SPH] Apply CFL multiplier at the end of the CFL computation#1883tdavidcl wants to merge 2 commits into
Conversation
|
Thanks @tdavidcl for opening this PR! You can do multiple things directly here: Once the workflow completes a message will appear displaying informations related to the run. Also the PR gets automatically reviewed by gemini, you can: |
There was a problem hiding this comment.
Code Review
This pull request refactors the CFL timestep calculation in Solver.cpp by moving the multiplication of cfl_multiplier from the individual particle-level calculations to the final next_cfl computation. A review comment highlights a regression caused by this change: the individual particle timesteps stored in cfl_dt remain unscaled, which leads to incorrect values being saved to the dt_part field if should_save_dt_to_fields() is enabled. It is recommended to scale these values before they are saved.
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.
| next_cfl | ||
| = sham::min(hydro_cfl, sink_sink_cfl) * solver_config.time_state.cfl_multiplier; |
There was a problem hiding this comment.
By moving the multiplication of cfl_multiplier to the very end of the CFL computation (i.e., only applying it to next_cfl), the individual particle timesteps stored in cfl_dt remain unscaled.
If solver_config.should_save_dt_to_fields() is enabled, these unscaled raw CFL timesteps are copied directly to the dt_part field (lines 2583-2594). This is a regression/bug, as dt_part should store the actual scaled timesteps used by the solver.
To fix this, the dt_part field (or cfl_dt after rank_dt is computed) should be scaled by solver_config.time_state.cfl_multiplier before being saved.
Workflow reportworkflow report corresponding to commit d2e5fee Pre-commit check reportPre-commit check: ✅ Test pipeline can run. Clang-tidy diff reportDoxygen diff with
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
No description provided.