Cleanup of CATKE and TKE previous_velocities#5483
Cleanup of CATKE and TKE previous_velocities#5483simone-silvestri wants to merge 3 commits intomainfrom
Conversation
| launch!(arch, grid, :xyz, | ||
| _rk_substep_turbulent_kinetic_energy!, | ||
| Le, σe⁻, grid, closure, | ||
| model.velocities, model.transport_velocities, |
There was a problem hiding this comment.
which one of these is the velocity at n and which one is n+1? I'm confused about this...
| Le, Lϵ, | ||
| grid, closure, | ||
| model.velocities, previous_velocities, | ||
| model.velocities, model.transport_velocities, |
There was a problem hiding this comment.
This will have to change soon if we want to use this for Breeze (which does not have transport_velocities) . Not sure the best way to capture this, but basically this is an optimization which makes the closure implementation less general
There was a problem hiding this comment.
Also the NonhydrostaticModel does not have transport_velocities
glwagner
left a comment
glwagner
left a comment
There was a problem hiding this comment.
We should consider this carefully because it is a specific optimization that makes it so we can't use these closures either with the NonhydrostaticModel or Breeze.AtmosphereModel.
|
Right it would limit the usage to only the hydrostatic model. Let's keep it hanging a bit, then we see if we could take another route. |
2 participants
we do not need the
previous_velocitiesfor CATKE and TKEBased vertical diffusivities anymore because the velocities used to advect the tracers are in themselves "previous velocities".Therefore, this PR removes the extra memory allocation by passing the
transport_velocitiesto CATKE and TKE-based closure.I also removed the
active_cell_mapsince it is introduced automatically when launching the kernels with:xyzand:xyThis PR also removes the substepping of TKE for RK time discretization since the derivation was done for AB2 so the interaction with RK is not validated.