Skip to content
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 vegetation temperature weighting during phenology #1306

Merged
merged 2 commits into from
Jan 16, 2025

Conversation

rgknox
Copy link
Contributor

@rgknox rgknox commented Jan 9, 2025

Description:

This change improves how we calculate vegetation temperature during phenology. The previous method included bare ground patches in the mean. Bare ground temperatures are initialized with the column/site level air temperature, but those values are not as representative of using only patch level vegetation temperatures. This new method filters in only temperatures from patches with vegetation, and uses the canopy area as the weighting factor, instead of total patch area.

This should precede #1226

Collaborators:

@rosiealice

Expectation of Answer Changes:

Yes, this should change answers, at least for tests that are long enough to trigger phenology events.

Checklist

If this is your first time contributing, please read the CONTRIBUTING document.

All checklist items must be checked to enable merging this pull request:

Contributor

  • The in-code documentation has been updated with descriptive comments
  • The documentation has been assessed to determine if updates are necessary (NOT NECESSARY)

Integrator

  • FATES PASS/FAIL regression tests were run
  • Evaluation of test results for answer changes was performed and results provided

Test Results:

TBD

CTSM (or) E3SM (specify which) test hash-tag:

CTSM (or) E3SM (specify which) baseline hash-tag:

FATES baseline hash-tag:

Test Output:

@rgknox rgknox requested review from glemieux and rosiealice January 9, 2025 17:12
Copy link
Contributor

@glemieux glemieux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks straightforward.

@mpaiao mpaiao self-requested a review January 13, 2025 20:13
Copy link
Contributor

@ckoven ckoven left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. Thanks @rgknox!

Copy link
Contributor

@mpaiao mpaiao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for these edits @rgknox, they look good and reasonable to me too!

@rgknox rgknox merged commit a969384 into NGEET:main Jan 16, 2025
1 check was pending
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Ready to Integrate
Development

Successfully merging this pull request may close these issues.

4 participants