-
Notifications
You must be signed in to change notification settings - Fork 7
update surface fluxes and update dependencies #1646
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
Conversation
acbfc8c to
db01989
Compare
|
Is this PR going to allow using a skin temperature? |
I think we will mimimal changes in this PR to recover the current behavior of the coupler, as we want to merge this relatively fast. But we can allow using a skin temperature now. I want to do it in a separate PR. |
4176cdc to
beabf31
Compare
beabf31 to
5bac863
Compare
e0dfe48 to
543d5a9
Compare
fd68206 to
7224a10
Compare
46a1d70 to
2c5b4bd
Compare
juliasloan25
left a comment
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.
The src changes look reasonable to me! One thought is we might want to merge this after we resolve the nightly AMIP compat troubles, and figure out why it started failing with the larger coupling timestep
| F_turb_ρτxz = ρτxz, | ||
| F_turb_ρτyz = ρτyz, | ||
| F_sh = shf, | ||
| F_lh = lhf, | ||
| F_turb_moisture = evaporation, |
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.
should we rename these all to use the same variable names as SurfaceFluxes? Especially for F_turb_moisture -> evaporation
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'm fine with that. I guess it depends on whether the coupler wants some convention of variable names - e.g. we have F_turb_energy and F_turb_moisture. It will change many files though - maybe we can do it in a separate PR?
I was thinking about merging this: CliMA/ClimaAtmos.jl#4198 soon, as we want to merge it before working on another breaking change on thermodynamics in ClimaAtmos (and ClimaLand and ClimaCoupler). But it will break nightly AMIP. I can make a patch release of ClimaAtmos now, and a breaking release with the ClimaAtmos PR, so we can resolve the nightly AMIP compat with the patch release of ClimaAtmos? I'm not too worried about the larger coupling timestep, but am happy to wait for a few days. Kat and I were not sure it's correct for integrated land - e.g. if the land timestep is 8 minutes and coupling timestep is 32 minutes, does the atmosphere get the flux at the end of the four land timesteps? This will cause some significant errors in energy conservation, although I don't know whether it will make things more unstable. |
|
Actually, amip breaks this week. I'm leaning towards there might be some issue with ClimaLand v1.3. Let's talk next week about how to move forward? |
6e66394 to
9f2c696
Compare
0adcb24 to
44068dd
Compare
|
Dependency updates in this PR: |
|
CMIP runs out of GPU parameter memory on the P100s with this Oceananigans update. It still runs fine on A100s. I'll make the CI runs soft fail so we can at least continue working on AMIP while we figure out a solution for CMIP. |
89e502c to
9d1376b
Compare
1b71ca4 to
c249297
Compare
c249297 to
2b45037
Compare
2b45037 to
2889878
Compare
Purpose
Use the new interface in SurfaceFluxes v0.15 to calculate surface fluxes. For the bucket simulation, call the turbulent_fluxes! in ClimaLand directly. Also updates dependencies.
To-do
Content