-
Notifications
You must be signed in to change notification settings - Fork 240
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 calculations in model2netcdf.ED2()
on E files
#3129
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… then converts to list. Removes ddbh as a variable
dlebauer
reviewed
Mar 8, 2023
dlebauer
approved these changes
Mar 8, 2023
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
Mar 8, 2023
Just installed this on Welsch and getting this warning for every variable, so something's not right still:
Going back to a draft for now. |
Aariq
commented
Mar 9, 2023
Whooops, I guess |
…oop, model2netcdf.ED2 writes one .nc file per year)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This is a near total re-write of the
read_E_files()
function that extracts data from monthly .h5 files output by ED2. The re-write simplifies code by extracting variables with the same dimensions into data frames and joining them before doing any calculations or unit conversions. This simplifies the logic greatly (IMO).It also removes DDBH (cm/plant/yr) as one of the variables extracted from the -E- files, since I'm no longer sure it even makes sense to summarize across cohorts.
The time dimensions were getting calculated incorrectly for monthly -E- file data as well (#3069), and that is now fixed in this PR.
Motivation and Context
Previously, the conversions were operating on the false assumption that variables like NPP and transpiration were coming out of ED2 in per-area units (EDmodel/ED2#384). Also, there was previously no weighting by patch area when summing across cohorts and patches. See more in #3126
Review Time Estimate
Types of changes
Checklist: