Modularize the entropy viscosity regularization - #2515
Conversation
There was a problem hiding this comment.
Pull request overview
This PR modularises entropy-viscosity-based artificial viscosity by splitting (1) the AVM model that computes a viscosity field from (2) the viscous regularisation that applies it in the solver, and wiring this through a new simulation component.
Changes:
- Introduces
avm_model_t+avm_simcomp_tto compute and output an AVM viscosity field (currentlyentropy_viscosity). - Adds
viscous_regularization_t+artificial_viscosity_tand refactors the compressible Euler scheme to updateeffective_viscvia the new interface. - Moves/extends CPU + device backends for entropy viscosity and updates docs/examples accordingly.
Reviewed changes
Copilot reviewed 22 out of 30 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| src/viscous_regularization/viscous_regularization.f90 | Renames/specialises the base regularisation abstraction to viscous regularisation. |
| src/viscous_regularization/viscous_regularization_fctry.f90 | Factory for viscous regularisation implementations. |
| src/viscous_regularization/artificial_viscosity.f90 | New viscous-regularisation implementation that applies AVM viscosity to effective_visc. |
| src/viscous_regularization/avm_model/avm_model.f90 | New AVM model base type and registry field setup. |
| src/viscous_regularization/avm_model/avm_model_fctry.f90 | Factory/registration mechanism for AVM models. |
| src/viscous_regularization/avm_model/entropy_viscosity.f90 | Refactors entropy viscosity to be an AVM model (preprocess/compute split). |
| src/viscous_regularization/avm_model/bcknd/cpu/entropy_viscosity_cpu.f90 | New CPU backend module for entropy viscosity kernels. |
| src/viscous_regularization/avm_model/bcknd/device/entropy_viscosity_device.F90 | New device-dispatch backend for entropy viscosity kernels. |
| src/viscous_regularization/avm_model/bcknd/device/cuda/entropy_viscosity.cu | New CUDA kernel launcher implementations. |
| src/viscous_regularization/avm_model/bcknd/device/cuda/entropy_viscosity_kernel.h | New CUDA kernels (entropy residual/viscosity/max/clamp/smooth). |
| src/viscous_regularization/avm_model/bcknd/device/hip/entropy_viscosity.hip | New HIP kernel launcher implementations. |
| src/viscous_regularization/avm_model/bcknd/device/hip/entropy_viscosity_kernel.h | New HIP kernels (entropy residual/viscosity/max/clamp/smooth). |
| src/viscous_regularization/avm_model/bcknd/device/opencl/entropy_viscosity.c | New OpenCL kernel launcher implementations. |
| src/viscous_regularization/avm_model/bcknd/device/opencl/entropy_viscosity_kernel.cl | New OpenCL kernels (entropy residual/viscosity/max/clamp/smooth). |
| src/simulation_components/avm_simcomp.f90 | New simcomp to drive AVM model preprocess/compute and output. |
| src/simulation_components/simulation_component_fctry.f90 | Registers the new avm_model simcomp type. |
| src/fluid/fluid_scheme_compressible_euler.f90 | Switches Euler scheme from embedded regularisation to viscous_regularization update. |
| src/fluid/euler_res.f90 | Updates RHS interface to remove h argument. |
| src/fluid/bcknd/cpu/euler_res_cpu.f90 | Updates CPU RHS implementation signatures to match new interface. |
| src/fluid/bcknd/device/euler_res_device.F90 | Updates device RHS implementation signatures to match new interface. |
| src/Makefile.am | Updates build lists to add new AVM/regularisation sources and remove old ones. |
| src/.depends | Regenerated dependency file reflecting new modules/paths. |
| src/.depends_device | Regenerated device dependency file reflecting new module paths. |
| examples/euler_tgv/euler_tgv.case | Updates example to use avm_model simcomp + viscous_regularization config. |
| examples/euler_2d_smooth/euler_2d_smooth.case | Updates example to use avm_model simcomp + viscous_regularization config. |
| examples/euler_2d_forward_facing_step/step.case | Updates example to use avm_model simcomp + viscous_regularization config. |
| examples/euler_2d_cylinder/euler_2d_cylinder.case | Updates example to use avm_model simcomp + viscous_regularization config. |
| examples/euler_1d_sod/sod.case | Updates example to use avm_model simcomp + viscous_regularization config. |
| doc/pages/user-guide/simcomps.md | Documents the new avm_model simcomp and its configuration. |
| doc/pages/user-guide/case-file.md | Documents viscous regularisation configuration and updates compressible example. |
Comments suppressed due to low confidence (4)
src/viscous_regularization/avm_model/entropy_viscosity.f90:127
- The
select type (fluid => case%fluid)block has noclass default/error path. Ifcase%fluidis notfluid_scheme_compressible_t, the field pointers (S/u/v/w/max_wave_speed/msh/Xh/gs) remain null and later calls will dereference them. Add an explicit default case that errors out (or handle additional fluid types) to avoid null pointer crashes.
src/viscous_regularization/avm_model/entropy_viscosity.f90:119 c_avisc_lowdefaults to 1.0 here, but the user-guide documentation (and prior compressible numerics defaults) state 0.5. Either update the code default back to 0.5 or update the documentation consistently; otherwise users will see unexpected behaviour changes when omitting the parameter.
src/viscous_regularization/viscous_regularization.f90:112viscous_regularization_thas an allocatable componentreg_coeff_name, butfree_basedoes not deallocate it. Base destructors should deallocate allocatable components (and nullify pointer components) to avoid leaks/stale state on reinitialisation.
src/viscous_regularization/avm_model/entropy_viscosity.f90:114reg_coeff_nameis a local allocatable that becomes allocated via json_get_or_default, but it is not deallocated before returning. Project guidelines require explicit deallocation of local allocatables allocated within a subroutine.
|
Avm_model become artificial viscosity method model. I kinda feel like u should choose either model or method and just write ot out, for example, artificial_viscosity_method |
|
I will rebase it after the compressible Navier-Stokes PR is merged :) |
Thank you for that :) I hope it will not be a hassle. |
e5247ec to
f8f010f
Compare
|
Now it's fixed with the latest compressible ns push |
timofeymukha
left a comment
There was a problem hiding this comment.
I think is all fine basically, just some comments.
The axhelm edit makes me scared...
It was me who forced pushed the buggy vector version a week or so ago |
4ad30cf to
4ff58f5
Compare
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Agent-Logs-Url: https://github.com/ExtremeFLOW/neko/sessions/764b8bb4-56f2-4d24-88e6-058a628fd185 Co-authored-by: Shiyu-Sandy-Du <115027341+Shiyu-Sandy-Du@users.noreply.github.com>
21f03c0 to
f851e68
Compare
|
@shiyu-du-dsy It seems Codex found some important stuff!
|
… the latest step.
|
Note that in the current version, the restart will re-compute the entropy viscosity based on the latest state in 1st order. For a real clean restart, the p_lag and rho_lag should be included in the check point file as well. However noting that restart is not in effect in the compressible solver, I would like to keep the issue to a fixing PR for the restart of the compressible solver. |
This PR separate entropy viscosity from the Euler solver by doing the following steps:
avm_simcomp_tfor the model for the artificial viscosity method (AVM), which handle the operation of the new typeavm_t. Nowentropy_viscosity_tis the only one derived type ofavm_t. This object is responsible for compute the entropy viscosity to be used at each time step.viscous_regularization_tto apply the artificial viscosity to the equation. So far there is only one derived typeartificial_viscosity_t, but lately I will add the spectral vanishing viscosity (SVV) as another child.To do list:
And regarding other regularization method such as GJP and HPF, since they do not act through the viscous term, we could keep them in the source term. (in the pnpn solver, viscous term is treated implicitly, therefore needs extra treatment)