-
Notifications
You must be signed in to change notification settings - Fork 16
Consolidating whole atmosphere calculations into single module #115
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
base: wam_dev
Are you sure you want to change the base?
Conversation
Prefer the new constructor of `QuantityFactory` over the deprecated call to `QuantityFactory.from_backend(...)`. This removes a bunch of deprecation warnings in tests. See NOAA-GFDL/NDSL#228 for context.
On `Quantities`, we have currently allow both `backend` and `gt4py_backend` where the later is deprecated and about to be removed. See NOAA-GFDL/NDSL#312 and NOAA-GFDL/NDSL#314 for context.
… DryConvectiveAdjustment
…update refactor: use new constructor of `QuantityFactory`
…update refactor: prefer `backend` over `gt4py_backend` on quantities
pyfv3/stencils/wam.py
Outdated
| delz: FloatField, | ||
| ): | ||
| self.constructed_adjust_gravity_stencil(grav_var, grav_var_h, phis, delz) | ||
| self.constructed_neg_rdgas_div_gravity_stencil(rdg, grav_var) |
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.
We talked offline about this, but just putting what I remember here. I know I wanted us to consider more using classes for the stencils, but I'm now leaning more towards just putting them all together without the classes, similar to the basic_operations.py in NDSL. Only for us, we'd have some sort of "wam operations" version. [Edit: I forgot to put that these two lines are only rarely used together. That neg rdgas div gravity is more often used separately, so this particular class might not be that useful.]
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 if we are able to call both the adjust_gravity and neg_rdgas_div_gravity stencils simultaneously, and not only when the whole atmosphere option is selected, it would make the most sense to have the class option for calling them. I included it in this PR so that we could see what it could look like. If we decide to always use the whole atmosphere option, then the class would be the way to go IMHO.
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 have my preference, but it's not a strong one. So, I'm fine with going this way.
Description
This PR introduces all current changes suggested by PR 395 in
gfdl_atmos_cubed_spherepertaining to the use of a variable gravity object to enable whole atmosphere calculations. It also consolidates previously introduced whole atmosphere calculation related stencils to a single module file and creates a class for calling these stencils (which has not been fully implemented in this PR, but discussions regarding its use are currently under way). Also, the calls for averaging the gravity and calculating rdg during the first timestep of the acoustic loop have been removed fromAcousticDynamicsas they are also done within the call toDynamicalCore.This PR also updates the
wam_devbranch with changes fromdevelop.Fixes # (issue)
If this is a hotfix to a released version, please specify it
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Please also note
any relevant details for your test configuration (e.g. compiler, OS). Include
enough information so someone can reproduce your tests.
Checklist: