add climaparams to update_aux #974
Conversation
costachris
force-pushed
the
cc/integrate_climaparams
branch
from
b8a8113
to
4de63d2
Compare
costachris
force-pushed
the
cc/integrate_climaparams
branch
from
4de63d2
to
4108449
Compare
There was a problem hiding this comment.
Hi Costa - this looks great!
3 points.
- As we now do our type enforcement when we get parameter values out of the toml struct, we no longer should need any of these
alpha::FTorFT(alpha(param_set))type expressions. These can be removed so we just callalpha = param_set.alpha. - For now i think it will be good practice to always give
param_seta type: some are missing in e.g.entr_detr.jlfunction definitions. - In theory we should have no calls to something like
param_set.box.parameter(only everparam_set.parameter). Where these appear - this indicates that either the dispatching is off (e.g. the code should wrap the code in a new function), or maybe the parameter needs to be put inparam_set- we can deal with these in a case-by-case basis offline.
| @@ -1,7 +1,7 @@ | |||
| #### Entrainment-Detrainment kernels | |||
|
|
|||
| function compute_turbulent_entrainment(param_set, a_up::FT, w_up::FT, tke::FT, H_up::FT) where {FT} | |||
| @@ -15,7 +15,7 @@ end | |||
| function compute_inverse_timescale(param_set, b_up::FT, b_en::FT, w_up::FT, w_en::FT, tke::FT) where {FT} | |||
| @@ -25,7 +25,8 @@ end | |||
|
|
|||
| function get_Δw(param_set, w_up::FT, w_en::FT) where {FT} | |||
| molmass_ratio::FT = CPP.molmass_ratio(param_set) | ||
| vkc::FT = CPSGS.von_karman_const(param_set) | ||
| function mixing_length(param_set::EDMFPS, ml_model::MinDisspLen{FT}) where {FT} | ||
| c_m::FT = param_set.c_m |
There was a problem hiding this comment.
Our param_set object automatically types all the parameters into FT thus you should remove all ::FT here.
| @@ -1,8 +1,8 @@ | |||
| #### Non-dimensional Entrainment-Detrainment functions | |||
| function max_area_limiter(param_set, max_area, a_up) | |||
| FT = eltype(a_up) | |||
There was a problem hiding this comment.
Again, we already type our parameters in param_set. We should be able to remove all the FT(param_set) and just have param_set.
| μ_0 = FT(CPEDMF.μ_0(param_set)) | ||
| β = FT(CPEDMF.β(param_set)) | ||
| χ = FT(CPEDMF.χ(param_set)) | ||
| c_ε = FT(param_set.c_ε) |
| updraft_top = compute_updraft_top(grid, state, i) | ||
| return max(updraft_top, H_up_min) | ||
| end | ||
|
|
||
| function compute_up_stoch_tendencies!(edmf::EDMFModel, grid::Grid, state::State, param_set::APS, surf::SurfaceBase) | ||
| function compute_up_stoch_tendencies!(edmf::EDMFModel, grid::Grid, state::State, param_set::EDMFPS, surf::SurfaceBase) |
There was a problem hiding this comment.
This should be param_set::AbstractStochasticEntrainmentClosureParameters (or some abbreviation)?
|
Can we first get CP 62 in mergeable shape? |
|
@trontrytel is working on internalizing / isolating TC's ClimaParameters in #1000, so we should be able to revert all of the source code changes except for the parameter boxes, and only focus on the parameter boxes in this PR. |
|
@charleskawczynski The internal climaparameters do not actually help us. They are a method of going from a Hierarchical storage of CP (currently in CP) to a flat box in the code source (ICP). The new climaparameters interface is the opposite, going from flat storage of CP (toml files) to a hierarchical deployment in the code source (Parameter boxes). [Besides we are effectively done already] |
|
Superseded by #1192. |
Use parameter boxes in
update_aux.jland propagate down where needed