Add more infrastructure for water tracers#82
Merged
Conversation
Since this is just used for unit testing, this is a reasonable setting.
The main motivation for this is that we can then avoid a dependency on shr_string_withoutSuffix in CMEPS by wrapping / stubbing this new shr_wtracers subroutine. But this also has the benefit of making the intent more clear for callers of this (as compared with directly calling shr_string_withoutSuffix).
Written by Claude Code, reviewed by myself.
Since the naming convention is to have "_wtracers" last, we're also changing the dimension ordering convention to be consistent with that.
Member
Author
|
I think this is basically ready, but I still want to do some more extensive testing, so marking this as a draft. |
billsacks
commented
Mar 3, 2026
By changing a division to a multiplication we can remove the pre-check for values being 0. Thanks to Keith Lindsay for the suggestion.
Member
Author
|
I'm finally(!) returning to this. I have run some testing (noted in my edited top-level comment above) and have looked over the changes once more myself. @nusbaume this is finally ready for your review whenever you get a chance. |
Written by Claude, reviewed by myself.
nusbaume
requested changes
May 1, 2026
Contributor
nusbaume
left a comment
nusbaume
left a comment
There was a problem hiding this comment.
Thanks for the new routine and tests @billsacks! I have some questions and change requests, but of course please let me know if you disagree with any of them.
Member
Author
|
@nusbaume - thank you very much for your review, and I'm sorry for my slow reply! To summarize my response to your comments: I'll at least make one change you requested: adding more information in shr_sys_abort messages. There are three changes that you asked about for which I have explained my rationale but am happy to change if you would still prefer to see changes after reading my rationale:
Let me know your thoughts on these. Also happy to discuss in person or on a call if you'd like. |
Claude made the changes, I gave them a cursory review.
nusbaume
approved these changes
May 14, 2026
Contributor
nusbaume
left a comment
nusbaume
left a comment
There was a problem hiding this comment.
Thanks for responding to my questions and suggestions @billsacks! Everything looks good to me now.
billsacks
added a commit
to ESCOMP/CMEPS
that referenced
this pull request
May 15, 2026
Add water tracers ### Description of changes Core changes needed to add water tracers in CESM, for the sake of water isotopes and other purposes. Major pieces of this include: - XML and config variables for defining water tracers - Addition of all necessary water tracers in esmFldsExchange_cesm_mod and fd_cesm.yaml. These use an ungridded dimension for water tracers. - Calls to a routine to check that test tracers maintain their initial ratios. This ensures that tracers are being treated the same as their bulk counterparts (in terms of mapping, merging, etc.). - Addition of water tracer fields in med_test_comps, so this tracer infrastructure can be verified in an X compset test - Various other changes to some of the med_test_comps to better exercise CMEPS functionality, especially for the lnd -> glc mapping; also added some fields from GLC and to ROF in med_test_comps: Fgrg_rofi and Fgrg_rofl Some pieces are not yet implemented: - Including water tracers in the custom lnd->rof mapping - Including water tracers/isotopes in atm-ocn flux calculations - Including water tracers in the budget tables (med_diag_mod) Depends on ESCOMP/CESM_share#82 ### Specific notes Contributors other than yourself, if any: CMEPS Issues Fixed (include github issue #): - Resolves #583 (though this turned out to be much more extensive than simply reviewing and updating what had been in place before) - Resolves #601 Are changes expected to change answers? (specify if bfb, different at roundoff, more substantial) Changes answers for X compsets (due to some changed counter logic and maybe other things); should be bit-for-bit for all other compsets Any User Interface Changes (namelist or namelist defaults changes)? Adds xml and config variables controlling water tracers ### Testing performed In the context of cesm3_0_alpha08o, with this branch along with ESCOMP/CESM_share#82, ran CESM prealpha tests on derecho and izumi, and aux_glc tests on derecho, all with baseline comparisons. All failures also failed in the baseline except for `SMS_D_Ln9_P1536x1.ne0CONUSne30x8_ne0CONUSne30x8_mt12.FHIST.derecho_intel.cam-outfrq9s` (which seemed to fail due to ESCOMP/CTSM#3772, which is a sporadic issue), and all baseline comparisons passed. (Note: full testing was done before final commits addressing review points; only minimal additional testing was done after those final commits.)
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Add more infrastructure for water tracers, needed by CMEPS.
The biggest addition here is a routine to check tracer ratios against expectations, with associated unit tests.
There are also a few other small additions to shr_wtracers_mod as well as some other additions needed for the unit test build.
Testing done: In the context of cesm3_0_alpha08o, with this branch along with ESCOMP/CMEPS#638, ran CESM prealpha tests on derecho and izumi, and aux_glc tests on derecho, all with baseline comparisons. All failures also failed in the baseline except for
SMS_D_Ln9_P1536x1.ne0CONUSne30x8_ne0CONUSne30x8_mt12.FHIST.derecho_intel.cam-outfrq9s(which seemed to fail due to ESCOMP/CTSM#3772, which is a sporadic issue), and all baseline comparisons passed. (Note: full testing was done before final commits addressing review points; only minimal additional testing was done after those final commits.)