-
Notifications
You must be signed in to change notification settings - Fork 415
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
Fix dz_node and node_depth in image statefile #657
Fix dz_node and node_depth in image statefile #657
Conversation
variables should be spatially distributed
output state file, image driver
Do we need to store them or do we just need to make sure that they are calculated every time the model starts? |
@bartnijssen Currently they are already calculated every time the model starts, and then in image driver it further checks if the calculated values match those in the initial state file (classic driver does not do the check; it always use those saved in the initial state file). I guess for model running purpose, we don't have to store them; for practical purpose (user wants to know node depth info), it might be informative - or we could write it out to history file if user outputs node-related variables. |
@bartnijssen and @yixinmao - here's my perspective, we want to write/read coordinate variables like
|
@jhamman So after the fix in this PR, image driver is doing what you proposed. Should we add the same check for classic driver as well? |
@yixinmao - no. The benefit of using netCDF to store the image driver state is there is a clear and concise way to keep metadata with the state variables. The classic driver doesn't really fit that bill so I would not suggest extending this beyond the image driver. |
@jhamman OK - the same two variables ( |
@yixinmao : Is this one ready to merge or are we waiting for something (other than me) |
@bartnijssen this should be ready to merge once someone takes a quick review. @jhamman @dgergel could you take a brief look? Most changes shown are due to indenting. |
LGTM. After @dgergel takes a look, I think this can go in. (please "squash and merge" though) |
@yixinmao and @jhamman this looks good to me. @bartnijssen, ready to merge. |
In image driver,
dz_node
andnode_depth
was assumed to be not spatially-distributed, which was wrong. Now output spatially distributeddz_node
andnode_depth
in the output state file; also, when reading in an initial state file, check these variables at all grid cells to see if they match the values calculated from soil parameters.