-
Notifications
You must be signed in to change notification settings - Fork 360
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
Turns on debugging for multiple land tests. #5599
Conversation
Creates a new test suite, `e3sm_land_debug`, in which multiple existing land tests are moved with debugging turned on.
6447b47
to
4cfeb91
Compare
@@ -376,6 +376,7 @@ subroutine PhenologyInit(bounds) | |||
! average 2m temp. | |||
! crit_onset_gdd = 150.0 ! c3 grass value | |||
! crit_onset_gdd = 1000.0 ! c4 grass value | |||
crit_onset_fdd=PhenolParamsInst%crit_onset_fdd |
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.
Is this change related to running a debug test or an open issue?
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.
I don't think it should be removed if it's tangential to the debug tests -- just thinking the PR description should mention it.
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.
This fix is needed to allow the few BGC tests to pass when debugging is turned on.
Testing this PR after a merge to next, I encountered a FAIL with Seems likely due to PR #5604 and the variable that is possibly
|
5dbbe95
to
469b9e3
Compare
For now, I have reverted the ELMFATES.chrysalis_intel.elm-fates_satphen test to be non-debugging. |
@bishtgautam I also found DIFFs in the unchanged land tests that only occurs when I merge this pr to the current next. Perhaps, a consequence of your change to PhenologyMod? Here are the tests:
|
@peterdschwartz You are correct. I will change the description and label of this PR to non-BFB. |
Merged to next -- only 3 existing land tests should DIFF and 7 new debug tests will need new baselines generated. |
I'll setup an issue on this FATES fail. Thanks for reporting. I believe that the issue is this variable is not being set to something meaningful: |
Creates a new test suite, e3sm_land_debug, in which multiple existing land tests
are moved with debugging turned on.
DIFFs for some existing land tests and replaces some land tests with DEBUG versions
[non-BFB]