-
Notifications
You must be signed in to change notification settings - Fork 5
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
update to atmos v0.20, land v0.7, etc #598
Conversation
4c71623
to
2145e5e
Compare
@@ -32,6 +32,9 @@ docs/src/generated/ | |||
!experiments/ClimaCore/**/Manifest.toml | |||
!perf/Manifest.toml | |||
|
|||
# Output | |||
output/ |
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.
before we checked in output/
so we didn't need this (output dir removed in #588)
4f8d5fa
to
0411393
Compare
d2f92bc
to
f5a3774
Compare
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.
LGTM, thank you, @juliasloan25 ! It's great to see some of the parameter simplifications! I just had a comment on energy flux passing.
I'm also seeing some of the conservation checks failed... looking into it now |
end | ||
function update_field!(sim::BucketSimulation, ::Val{:turbulent_moisture_flux}, field) | ||
ρ_liq = (LSMP.ρ_cloud_liq(sim.model.parameters.earth_param_set)) | ||
parent(sim.integrator.p.bucket.evaporation) .= parent(field ./ ρ_liq) # TODO: account for sublimation | ||
parent(sim.integrator.p.bucket.turbulent_fluxes.vapor_flux) .= parent(field ./ ρ_liq) # TODO: account for sublimation | ||
end |
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.
Re conservation, does ClimaLand still use m / s for sim.integrator.p.bucket.turbulent_fluxes.vapor_flux
or is it now kg / m2 / s ?
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.
it is still m/s...that is the last thing we need to do (across all land models) to be consistent with the flux conventions we agreed on with MIT.
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.
Sounds good. There's no huge rush on it, but pls let us know when it changes. Thank you for confirming! :)
2527090
to
126954d
Compare
70ba922
to
ff73a54
Compare
ff73a54
to
ce91c6d
Compare
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.
LGTM!
Purpose
update to latest dependencies
note: ClimaLSM v0.7.0 moves almost all bucket functions dispatching on
CoupledAtmosphere
into ClimaLSM so we can remove them here. However, two functions still need to be defined in ClimaCoupler. I've documented them here: #601note 2: this update causes functions using the
allskywithclear
radiation option to be unstable. See #602 for more info.closes #593