Refactor med_methods_FB_init to allow handling of water tracer fields #620

billsacks · 2025-12-24T02:03:45Z

Description of changes

Rather than having med_methods_FB_init accept one of fieldNameList or
STflds, now it accepts an array of med_field_info_type objects.
med_field_info_type is defined in a new module that also includes some
creation methods, making calls to med_methods_FB_init a two-step
process: first create field_info_array, then call med_methods_FB_init
with this field_info_array.

A key new feature is that the creation of a field_info_array from
field_names will assume a single ungridded dimension with size given by
shr_wtracers_get_num_tracers for any field with the water tracer suffix.
This was the main motivation for the refactor here.

Some specific notes about the changes in this commit:

Behavior change: if there are only scalar or blank fields, we do some
unnecessary work now (retrieving the mesh that we won't end up ever
using)
I think I got the gridToFieldMap right: before it was hard-coded to 2,
but I think that was only right for a single ungridded dimension; here
I'm generalizing that for multiple ungridded dimensions

This depends on the CESM_share changes in ESCOMP/CESM_share#78 and ESCOMP/CESM_share#80

Specific notes

Contributors other than yourself, if any:

CMEPS Issues Fixed (include github issue #):

Are changes expected to change answers? (specify if bfb, different at roundoff, more substantial) No - bfb

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

Testing performed

Please describe the tests along with the target model and machine(s)
If possible, please also added hashes that were used in the testing

Ran a subset of the CESM prealpha tests with baseline comparisons, using cesm3_0_alpha08a with this CMEPS branch and the share branch from ESCOMP/CESM_share#78:

ERS_D_Ld3.ne30pg3_t232.B1850C_LTso.derecho_intel.allactive-defaultio
ERI.ne30pg3_t232.B1850C_LTso.derecho_intel.allactive-defaultio
ERC_D_Ln9.ne30pg3_ne30pg3_mt232.FHISTC_LTso.derecho_intel.cam-outfrq9s
ERS_Ly7.f09_g17_gris4.T1850Gg.derecho_intel
SMS_Lm13.f10_f10_mg37.I1850Clm50SpG.derecho_intel
MULTINOAIS_Ly2.f10_f10_ais8gris4_mg37.I1850Clm50SpRsGag.derecho_intel.cism-change_params
SMS_D_Ld1.ne30pg3_t232.I1850Clm50BgcSpinup.derecho_intel.clm-cplhist
ERI.TL319_t232.G_JRA.derecho_intel.cice-default
SMS_D.TL319_t232.G_JRA_RYF.derecho_intel
SMS_Ld40.TL319_t232.C_JRA.derecho_intel

All tests pass and are bit-for-bit with the baselines.

There were NLCOMP failures in some tests; from spot-checking, the NLCOMP failures all seem to be in MOM files. Nothing in this PR or the accompanying share PR could change the namelists, so I assume these are a pre-existing issue:

    FAIL ERI.ne30pg3_t232.B1850C_LTso.derecho_intel.allactive-defaultio NLCOMP
    FAIL ERI.TL319_t232.G_JRA.derecho_intel.cice-default NLCOMP
    FAIL ERS_D_Ld3.ne30pg3_t232.B1850C_LTso.derecho_intel.allactive-defaultio NLCOMP
    FAIL SMS_D.TL319_t232.G_JRA_RYF.derecho_intel NLCOMP
    FAIL SMS_Ld40.TL319_t232.C_JRA.derecho_intel NLCOMP

Rather than having med_methods_FB_init accept one of fieldNameList or STflds, now it accepts an array of med_field_info_type objects. med_field_info_type is defined in a new module that also includes some creation methods, making calls to med_methods_FB_init a two-step process: first create field_info_array, then call med_methods_FB_init with this field_info_array. A key new feature is that the creation of a field_info_array from field_names will assume a single ungridded dimension with size given by shr_wtracers_get_num_tracers for any field with the water tracer suffix. This was the main motivation for the refactor here. Some specific notes about the changes in this commit: - Behavior change: if there are only scalar or blank fields, we do some unnecessary work now (retrieving the mesh that we won't end up ever using) - I think I got the gridToFieldMap right: before it was hard-coded to 2, but I think that was only right for a single ungridded dimension; here I'm generalizing that for multiple ungridded dimensions

We haven't set that up here; we'll fix this in an upcoming set of changes.

billsacks · 2025-12-24T02:14:32Z

The build-cmeps workflow will fail here until ESCOMP/CESM_share#78 is merged... then we can try rerunning it at that point.

First iteration of adding water tracer infrastructure This PR adds `shr_wtracers_mod` to coordinate adding arbitrary water tracers across CESM. This also adds `shr_string_withoutSuffix`, which can be used to determine if a given field is a water tracer field, and if so, return the name of the field without the water tracer suffix. I'll be adding more functionality in a follow-up PR, but some of the changes here are needed for ESCOMP/CMEPS#620.

jedwards4b · 2025-12-31T15:15:11Z

Perhaps if the water isotope code is something for cesm only and that will not be used by UFS or other external CDEPS users its usage should be inside #ifdef CESM blocks?

jedwards4b · 2025-12-31T16:04:31Z

Something like this:

diff --git a/mediator/med_field_info_mod.F90 b/mediator/med_field_info_mod.F90
index 3856fa93..a0b9cb18 100644
--- a/mediator/med_field_info_mod.F90
+++ b/mediator/med_field_info_mod.F90
@@ -10,8 +10,9 @@ module med_field_info_mod
   use med_utils_mod, only : ChkErr => med_utils_ChkErr
   use shr_log_mod, only : shr_log_error
   use shr_string_mod, only : shr_string_withoutSuffix
+#ifdef CESMCOUPLED
   use shr_wtracers_mod, only : WTRACERS_SUFFIX
-
+#endif
   implicit none
   private
 
@@ -36,6 +37,9 @@ module med_field_info_mod
      integer, allocatable :: ungridded_ubound(:)
   end type med_field_info_type
 
+#ifndef CESMCOUPLED
+  character(len=5), parameter WTRACERS_SUFFIX="undef"
+#endif
   character(len=*),parameter :: u_FILE_u = &
      __FILE__

mvertens · 2025-12-31T13:18:07Z

mediator/med.F90

                      STgeom=is_local%wrap%NStateImp(n2), &
-                      STflds=is_local%wrap%NStateImp(n1), &
                      name='FBImp'//trim(compname(n1))//'_'//trim(compname(n2)), rc=rc)
               end if


Don't you want to deallocate field_info_array here - before you allocate it again below. This is a bit tricky since the field_info_array is actually being allocated in the calls to med_field_info_array_from_XXX.

This comment applies to every time med_field_info_array_from_XXX is called in the various routines.

I had wondered the same thing. But I think it's okay as is: According to Modern Fortran Explained, in the section on "Allocatable dummy arguments": "If the intent is out and the array is allocated on entry, it becomes deallocated."

@billsacks - looks good.

mvertens · 2025-12-31T13:20:23Z

mediator/med.F90

            end if
            call FB_init(is_local%wrap%FBMed_ocnalb_o, is_local%wrap%flds_scalar_name, &
-                 STgeom=is_local%wrap%NStateImp(compocn), fieldnamelist=fldnames, name='FBMed_ocnalb_o', rc=rc)
+                 field_info_array = field_info_array, STgeom=is_local%wrap%NStateImp(compocn), name='FBMed_ocnalb_o', rc=rc)


Again - don't you want to add a deallocate call for field_info_array to accompany the deallocate fldnames call?

This way CESM can use shr_wtracers_mod while UFS uses a stub that removes this CESM_share dependency.

billsacks · 2025-12-31T23:11:25Z

I have pushed some commits that:

(1) Address requests from @mvertens from our discussion yesterday (I have not made changes per @mvertens 's above review - see my response to that review above... happy to discuss that more if you disagree with what I said there).

(2) Remove the new dependencies on share code. I have done this by introducing a small wrapper layer on top of shr_wtracers_mod. For CESM, this wraps shr_wtracers_mod; for UFS (and the standalone build, which uses the ufs directory), this stubs out the used function. In a future PR where I more fully implement the water tracer functionality, I'll build out these wrappers, wrapping/stubbing some additional functions.

@jedwards4b - I have added you as a reviewer, mainly thinking that you could review how I introduced these new wrappers: see wtracers_mod.F90 in the cesm and ufs directories, as well as my changes to buildexe.

Note that, for CESM, this depends on the CESM_share changes in ESCOMP/CESM_share#80.

jedwards4b

This makes sense to me but now you have two files of the same name which must have different contents instead of two files of the same name that must have the same contents. Is that any better?

billsacks · 2026-01-01T00:23:59Z

This makes sense to me but now you have two files of the same name which must have different contents instead of two files of the same name that must have the same contents. Is that any better?

Hmmm, you raise a good point, and you got me thinking more about this, but I do think this is better, because having two files with the same contents either requires continual effort to keep them in sync or confusion when they get out of sync and someone wonders if it matters that they differ (as has caused me confusion in similar situations in the past). Here it is hopefully clear that the version here is just a stub and so there's no expectation that it be kept in sync any more than is necessary (i.e., adding stubs for any new APIs that are used).

That said, I'm happy to discuss this further if you have concerns with this approach.

billsacks · 2026-01-01T00:30:45Z

This makes sense to me but now you have two files of the same name which must have different contents instead of two files of the same name that must have the same contents. Is that any better?

Hmmm, you raise a good point, and you got me thinking more about this, but I do think this is better, because having two files with the same contents either requires continual effort to keep them in sync or confusion when they get out of sync and someone wonders if it matters that they differ (as has caused me confusion in similar situations in the past). Here it is hopefully clear that the version here is just a stub and so there's no expectation that it be kept in sync any more than is necessary (i.e., adding stubs for any new APIs that are used).

That said, I'm happy to discuss this further if you have concerns with this approach.

Oh, and I forgot one other point that seems important at least for this specific case: I realized that, in some instances, I may actually want different logic for UFS than for CESM, because in UFS the tracer module won't be properly initialized, so it could actually help to have a stub version that does nothing and/or returns hard-coded values rather than running through the actual logic.

Add shr_wtracers_is_wtracer_field The main motivation of adding this is that it helps me break a dependency of CMEPS on CESM_share, which I'll illustrate in ESCOMP/CMEPS#620. But this also feels like it will lead to more self-documenting code, since `shr_wtracers_is_wtracer_field` is more clear than calling `shr_string_withoutSuffix` directly.

billsacks added 3 commits December 23, 2025 15:51

Remove some unused behavior from med_methods_FB_init

1921680

For now, hard-code n_tracers = 0

a39d47d

We haven't set that up here; we'll fix this in an upcoming set of changes.

billsacks requested a review from mvertens December 24, 2025 02:03

billsacks added 3 commits December 30, 2025 14:56

Add new file to CMakeLists.txt

e9dc860

Minor cleanup suggested in review

889368b

Rename a subroutine, as suggested in review

5a3d2b8

billsacks mentioned this pull request Dec 31, 2025

Rework the CMEPS GitHub Action build to use the CESM_share library instead of the share code in CDEPS #623

Open

mvertens requested changes Dec 31, 2025

View reviewed changes

billsacks added 2 commits December 31, 2025 14:56

Refactor to use new shr_wtracers_is_wtracer_field function

adca1fd

Add some comments

2ec86d3

billsacks mentioned this pull request Dec 31, 2025

Add shr_wtracers_is_wtracer_field ESCOMP/CESM_share#80

Merged

Wrap shr_wtracers_mod

d60fbeb

This way CESM can use shr_wtracers_mod while UFS uses a stub that removes this CESM_share dependency.

billsacks force-pushed the refactor_med_methods_fb_init branch from 2559bfa to d60fbeb Compare December 31, 2025 22:58

Remove unused variable

43e4594

billsacks requested review from jedwards4b and mvertens December 31, 2025 23:04

billsacks self-assigned this Dec 31, 2025

jedwards4b approved these changes Dec 31, 2025

View reviewed changes

mvertens approved these changes Jan 1, 2026

View reviewed changes

Refactor med_methods_FB_init to allow handling of water tracer fields #620

Are you sure you want to change the base?

Refactor med_methods_FB_init to allow handling of water tracer fields #620

Uh oh!

Conversation

billsacks commented Dec 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of changes

Specific notes

Testing performed

Uh oh!

billsacks commented Dec 24, 2025

Uh oh!

jedwards4b commented Dec 31, 2025

Uh oh!

jedwards4b commented Dec 31, 2025

Uh oh!

mvertens Dec 31, 2025

Choose a reason for hiding this comment

Uh oh!

mvertens Dec 31, 2025

Choose a reason for hiding this comment

Uh oh!

billsacks Dec 31, 2025

Choose a reason for hiding this comment

Uh oh!

mvertens Jan 1, 2026

Choose a reason for hiding this comment

Uh oh!

mvertens Dec 31, 2025

Choose a reason for hiding this comment

Uh oh!

billsacks commented Dec 31, 2025

Uh oh!

jedwards4b left a comment

Choose a reason for hiding this comment

Uh oh!

billsacks commented Jan 1, 2026

Uh oh!

billsacks commented Jan 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

billsacks commented Dec 24, 2025 •

edited

Loading