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

Always check finidat_interp_dest.status #2679

Merged
merged 4 commits into from
Aug 13, 2024

Conversation

samsrabin
Copy link
Collaborator

Description of changes

Adds a check for the existence of the finidat_interp_dest.status file.

Specific notes

Contributors other than yourself, if any: None

CTSM Issues Fixed (include github issue #):

Are answers expected to change (and if so in what way)? No

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

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

Testing performed, if any:

  • These commits solved the problem on the branch I initially encountered it on
  • I haven't yet done aux_clm testing; waiting for initial PR approval.

@samsrabin samsrabin added simple bfb bit-for-bit labels Aug 8, 2024
@samsrabin samsrabin added this to the ctsm6.0.0 (code freeze) milestone Aug 8, 2024
@samsrabin samsrabin self-assigned this Aug 8, 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.

Looks good @samsrabin

Copy link
Collaborator

@ekluzek ekluzek left a comment

Choose a reason for hiding this comment

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

Always good to move a simple thing into a subroutine that is then more modular and could allow a unit test to do be done.

If I understand the core change -- it's that the status is always going to be checked instead of only some of the time. Is that the right characterization @samsrabin? That also sounds like a good thing to do, and a nice usability improvement. I do think these files continue to sometimes cause problems for people and this should help.

@samsrabin
Copy link
Collaborator Author

Thanks, both! Adding the stalled label because testing will have to wait for Derecho to return to service.

And @ekluzek, yes, it's just adding an extra check (the one in apply_use_init_interp().

@samsrabin
Copy link
Collaborator Author

True differences were found in 4 tests so far:

SMS_D.1x1_brazil.I2000Clm60FatesSpCruRsGs.derecho_gnu.clm-FatesColdDryDepSatPhen
SMS_D.1x1_brazil.I2000Clm60FatesSpCruRsGs.derecho_gnu.clm-FatesColdMeganSatPhen
ERP_P64x2_Lm13.f10_f10_mg37.IHistClm60Bgc.derecho_intel.clm-monthly--clm-matrixcnOn_ignore_warnings (EXPECTED FAIL)
REP_P64x2_Lm13.f10_f10_mg37.IHistClm60Bgc.derecho_intel.clm-monthly--clm-matrixcnOn_ignore_warnings (EXPECTED FAIL)

The FATES ones were all in FATES_TRANSITION_MATRIX_LULU. Interestingly, they weren't all infinity as expected:

SMS_D.1x1_brazil.I2000Clm60FatesSpCruRsGs.derecho_gnu.clm-FatesColdDryDepSatPhen.C.0812-105150de_gnu
 RMS FATES_TRANSITION_MATRIX_LULU       Infinity            NORMALIZED    Infinity
 RMS FATES_TRANSITION_MATRIX_LULU       Infinity            NORMALIZED    Infinity
 RMS FATES_TRANSITION_MATRIX_LULU       Infinity            NORMALIZED    Infinity
 RMS FATES_TRANSITION_MATRIX_LULU       Infinity            NORMALIZED    Infinity
 RMS FATES_TRANSITION_MATRIX_LULU       Infinity            NORMALIZED    Infinity
 RMS FATES_TRANSITION_MATRIX_LULU       Infinity            NORMALIZED    Infinity

SMS_D.1x1_brazil.I2000Clm60FatesSpCruRsGs.derecho_gnu.clm-FatesColdMeganSatPhen.C.0812-105150de_gnu
 RMS FATES_TRANSITION_MATRIX_LULU     2.5489+147            NORMALIZED  1.5991-104
 RMS FATES_TRANSITION_MATRIX_LULU     2.5489+147            NORMALIZED  1.5991-104
 RMS FATES_TRANSITION_MATRIX_LULU     2.5489+147            NORMALIZED  1.5991-104
 RMS FATES_TRANSITION_MATRIX_LULU     2.5489+147            NORMALIZED  1.5991-104
 RMS FATES_TRANSITION_MATRIX_LULU     2.5489+147            NORMALIZED  1.5991-104
 RMS FATES_TRANSITION_MATRIX_LULU     2.5489+147            NORMALIZED  1.5991-104

But I think that's basically infinity, so I'm not worried about it.

Waiting for a few more tests to finish.

@slevis-lmwg
Copy link
Contributor

The LULU failures have an open issue that Greg is working on.

@samsrabin
Copy link
Collaborator Author

Yep, #2656. Also, the failed clm-matrixcnOn_ignore_warnings tests stem from #2619. So this is good to go—merging!

@samsrabin samsrabin merged commit 8252dc8 into ESCOMP:b4b-dev Aug 13, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bfb bit-for-bit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants