-
Notifications
You must be signed in to change notification settings - Fork 19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
New CloudMicrophysics parameters interface #2299
Conversation
precipitation_advection_tendency!(Yₜ, Y, p, colidx, precip_model) | ||
return nothing | ||
end | ||
#function precipitation_cache(Y, precip_model::Microphysics1Moment) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we use it anywhere. I would prefer to first update things properly for the 0-moment microphysics and the code up the 1-moment microphysics
src/parameters/create_parameters.jl
Outdated
subparam_structs = (; thermo_params), | ||
) | ||
|
||
toml_dict["τ_precip"]["value"] = FT(CA.time_to_seconds(parsed_args["dt"])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably a temporary solution? The alternative is for me to make the CloudMicrophysics structs be kwdef
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we discussed, using @kwdef
does not seem logical when default values are not being set! Updating the value for the dict key looks like the more natural thing to do!
src/parameters/Parameters.jl
Outdated
Base.broadcastable(param_set::ACAP) = tuple(param_set) | ||
Base.broadcastable(param_set::ATCP) = tuple(param_set) | ||
|
||
Base.@kwdef struct TurbulenceConvectionParameters{FT} <: ATCP |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if we want to keep edmf parameters in a separate struct or have them be part of ClimaAtmos parameters below. Right now things are mixed up a little. I can clean it up in a separate PR, once we decide @szy21
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At least right now we don't put microphysics
parameters inside turbconv
parameters so that we can create thermodynamics
parameters...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be good to have edmf parameters in a separate struct. We can do it after this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I think I'll do it before. Stay tuned ;)
881b12b
to
dc9cbcf
Compare
This PR:
CM.jl
parameters interface (currently on the main branch. We should have the release once we are confident that things for forCA.jl
)TurbulenceConvection/Parameters.jl
file