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

PPE change to sa_leaf in CanopyFluxesMod.F90 #2788

Merged
merged 5 commits into from
Oct 14, 2024
Merged

Conversation

olyson
Copy link
Contributor

@olyson olyson commented Sep 25, 2024

Description of changes

Add change to sa_leaf that was in PPE branch but is not on master

Specific notes

Contributors other than yourself, if any: @olyson

CTSM Issues Fixed (include github issue #):
Fixes #2777

Are answers expected to change (and if so in what way)? Yes. See #2777 for results.

Any User Interface Changes (namelist or namelist defaults changes)? No

Does this create a need to change or add documentation? Did you do so? Yes. No.

Testing performed, if any:
No test suite testing thus far.

@olyson olyson added bug something is working incorrectly PR status: awaiting review Work on this PR is paused while waiting for review. science Enhancement to or bug impacting science non-bfb Changes answers (incl. adding tests) labels Sep 25, 2024
@olyson olyson self-assigned this Sep 25, 2024
Copy link
Contributor

@slevis-lmwg slevis-lmwg left a comment

Choose a reason for hiding this comment

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

@olyson found this line missing from the recent PPE code that came to master. That recent code came in as b4b, while the modification here changes answers. Keith already evaluated the effect of this modification in #2777.

To me it seems a good idea to merge this quickly.

@slevis-lmwg slevis-lmwg removed the PR status: awaiting review Work on this PR is paused while waiting for review. label Sep 26, 2024
@samsrabin samsrabin removed their assignment Oct 3, 2024
@samsrabin
Copy link
Collaborator

@wwieder Removing my assignment, as with the change to our queue order I no longer care when this comes in ;-)

@slevis-lmwg

This comment was marked as resolved.

@slevis-lmwg
Copy link
Contributor

Starting aux_clm.

Copy link
Contributor Author

@olyson olyson left a comment

Choose a reason for hiding this comment

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

I think "larger than roundoff/same climate" is adequate, and maybe not put "X" in the checkboxes? Unless "significant" means the same thing as "larger than rounoff/same climate". I interpret significant as meaning different climate, which is not what I think we have here. Also, I suggest maybe include a direct link to the diagnostics, instead of indirectly through the issue?

@slevis-lmwg
Copy link
Contributor

I have preemptively updated the ChangeLog with aux_clm results.
Izumi is done and came back OK.
Derecho is in progress and looks promising this far.

@slevis-lmwg slevis-lmwg merged commit f726adb into ESCOMP:master Oct 14, 2024
2 checks passed
@slevis-lmwg slevis-lmwg deleted the saleaf branch October 14, 2024 23:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something is working incorrectly non-bfb Changes answers (incl. adding tests) science Enhancement to or bug impacting science
Projects
Status: Done (non release/external)
Status: Done
Development

Successfully merging this pull request may close these issues.

Missing sa_leaf code from PPE branch in CanopyFluxesMod.F90
3 participants