-
Notifications
You must be signed in to change notification settings - Fork 3
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
HostDynamics Driver #78
Conversation
…/haero into pbosler/host_driver
…ths; updated read_yaml_test.cpp.in to include the driver namespace; updated atm_conditions test to new parameters; updated read_yaml and smoke_test.yml to new parameters
namespace haero { | ||
namespace driver { | ||
|
||
void HostDynamics::init_from_interface_heights(std::vector<Real> z0, |
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.
Any reason you chose not to declarez0
as a const reference? Just curious.
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.
Yes, I want to force us to think like Homme, with vertical levels starting at model top and going down to the surface as the index k increases. So, in case a user puts z0 values in ascending order, I wanted a non-const copy to run std::reverse
on them.
@@ -1,11 +1,17 @@ | |||
include(EkatCreateUnitTest) | |||
|
|||
EkatCreateUnitTest(atm_conditions atm_condition_test.cpp LIBS haero_standalone ${HAERO_LIBRARIES}) | |||
EkatCreateUnitTest(atm_conditions atm_condition_test.cpp LIBS haero_standalone ${HAERO_LIBRARIES} |
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.
Strictly speaking (at least conceptually), ${HAERO_DRIVER_LIBRARIES}
should precede ${HAERO_LIBRARIES}
, since systems with a single-pass linker (like Linux) gather dependencies from earlier items and satisfy them with later ones, and arguably the driver depends on the library. But I think the only libraries we build into the driver and not the library are I/O-related, so it doesn't matter in this case.
writer.add_atm_state_data(atm, time_idx); | ||
|
||
writer.close(); | ||
} |
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 some point, it might be nice to have a reader for these kinds of tests that can verify properties of the written files. Not urgent, though.
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.
Nice job, Pete! I left some comments here and there, but overall it looks good to me. I didn't look very closely at the atmospherics part, so maybe others can inspect that more carefully.
A lot of this appears above the level of my current expertise, but can confirm that it builds/runs on my Mac for pack size 1 and 2, and the output appears to be as expected. |
…(p) invert each other.
Does anyone object to merging this PR? The driver in the main branch doesn't use our single-column indexing scheme, so it's out of date. I'd like to claim that our future PRs will probably be smaller and more incremental, since we've made a lot of important and significant decisions lately, but I know better than to promise that. :-) |
I didn't go through the entire PR but I think it is okay to merge this PR. The changes that I went through looks like a great addition to me. I would also prefer small incremental PRs but I know it is sometimes hard to do that. |
Summary
1D host dynamics implemented, with support for writing HOMME prognostic variables and Haero::Atmosphere state data to netCDF file.
To check it out: Run the new host dynamics unit test. It will create 2 netcdf files, one for height-initialized interfaces, the other for pressure -initialized interfaces. Each file contains 2 time steps: the initial time t=0 and t = period/2, to show the state after it's been vertically lifted to the furthest extent. To view the data, run
ncdump host_dynamics_test_zinit.nc
Features
The model is vertically Lagrangian, meaning that the geopotential surfaces (and height surfaces) move with the velocity of the vertical wind. If you need to analyze a fixed pressure level or fixed height level, we'll need to implement an interpolation scheme. Ekat already has a piecewise linear scheme built-in.
As demonstrated in the Host Dynamics unit test:
Questions / issues that came up
Haero::Atmosphere
do we want to keep height on interfaces, or would it be better to have it at level midpoints (so that every variable passed to physics is at a level midpoint)?qv/qvsat
, whereqv
is the water vapor mixing ratio andqvsat
is the saturation mixing ratio. Saturation mixing ratio computed with the Tetens equation from Soong-Ogura 1973, Appendix A eqn. (A1), which is also used in Klemp-Wilhelmson 1978 eqn. (2.11). Is there a better way to compute relative humidity?Documentation
Documentation is done, but needs to be integrated to the haero design doc; see #21, #71.
Developments & fixes included in this PR
Atmosphere
state containers for use with params. Doesn't build tendencies yet. Created Define Driver dynamics tendencies #77.read_yaml
unit test.NcWriter
to new column-based indexing scheme, added support forSpecies
andAtmosphere
.Model
support is likely something that we'll want forNcWriter
: created Add aerosol config support in NcWriter #76 .Closes #75.
Closes #70.
Closes #14.
Closes #19.
Testing
New test of host dynamics passes on Mac with pack size = 1, 2.