Conversation
Partially addresses ESCOMP#601
This will integrate better with existing infrastructure for processing colon-delimited lists.
Before this change, an empty WATER_TRACERS xml variable was treated as a length-1 list containing an empty string, rather than as a length-0 list.
It looks like these fields were removed from esmFldsExchange_cesm_mod at some point, but weren't removed from fd_cesm.yaml.
We can enable these checks once we add water tracers to ensure that water tracers are being handled consistently with bulk fluxes.
This reverts commit a39d47d. This capability is enabled on this branch so we can back out this temporary commit.
This means we are potentially getting n_tracers multiple times instead of just once, but it allows the code to work in the situation where we don't have any tracer fields and shr_wtracers_mod hasn't been initialized. (I'm not sure if this will arise in practice, but it might.)
This breaks the dependency on this share code for UFS.
This will make it more clear to readers that tracers are handled identically to bulk, and will also make it less likely for the two to get out of sync.
Per suggestion from Mariana Vertenstein
Since this field doesn't have a corresponding bulk field, we need to add some flexibility to the checker method.
Faxa_rain, Faxa_rainc, Faxa_rainl, Faxa_snow, Faxa_snowc, Faxa_snowl
Also add non-tracer fields for Forr_rofl_glc and Forr_rofi_glc, which hadn't previously been added.
Make ice_covered 0 some places. Make topo range from 0 to 3000. Code written by Claude Code (with substantial guidance and iteration from me), reviewed by myself.
This is needed for the smb renormalization
Since the naming convention is to have "_wtracers" last, we're also changing the dimension ordering convention to be consistent with that.
The lnd->glc mapping relies on particular values of coupling fields if we want to fully test the logic, so set some more reasonable values for the necessary fields.
|
I still want to do a more extensive review and testing of this myself, so for now marking this as draft. |
| <entry id="water_tracer_names" modify_via_xml="WATER_TRACERS" skip_default_entry="true"> | ||
| <type>char</type> | ||
| <category>water_tracers</category> | ||
| <group>ALLCOMP_attributes</group> |
There was a problem hiding this comment.
I'm putting the new variables in ALLCOMP_attributes. I'm not sure if this is right, or if these should go in DRIVER_attributes. I chose ALLCOMP_attributes just because there was more stuff (and more relevant stuff) there, but I'm not sure why I'd choose one over the other.
But actually, maybe ALLCOMP_attributes is needed given that MOM may need to access these rather than going through the share code??
@mvertens do you have guidance here?
There was a problem hiding this comment.
@billsacks - if any component needs this variable - then it should belong in ALLCOMP_attributes. I think I envisioned the DRIVER_attributes as having variables that are available on all processors but are specific to the driver itself (e.g. pio, etc).
I am not sure why the following variables
reprosum_diffmax
reprosum_recompute,
reprosum_use_ddpdd
wv_sat_scheme
wv_sat_table_spacing
wv_sat_transition_start
wv_sat_use_tables
are in the DRIVER_attributes and not in the ALLCOMP_attributes.
@jedwards4b - what are your thoughts on this?
They do the same thing but shr_wtracers_get_bulk_fieldname makes the purpose more clear.
It turns out that this is needed when running with CISM. This partially reverts the changes in 0f793bd.
| canonical_units: kg m-2 s-1 | ||
| description: lnd export to river | ||
| # | ||
| - standard_name: Flrl_rofdto |
There was a problem hiding this comment.
This removal of Flrl_rofdto looks weird in the diff - since it looks like I replaced Flrl_rofdto with Flrl_irrig_wtracers - but in fact this is intentional: I removed the no-longer-used Flrl_rofdto and separately added Flrl_irrig_wtracers.
| private :: med_io_def_var_with_atts | ||
| private :: med_io_read_1d_var |
There was a problem hiding this comment.
The changes in this module are not directly related to the addition of water tracers. Rather, they are a follow-on to the additions made to med_io_mod in #634. Those additions were for the sake of adding water tracers, so this is still somewhat related to this overall goal. However, I'd be open to moving these to a separate PR if desired.
| do ns = 1,is_local%wrap%num_icesheets | ||
| call fldbun_reset(toglc_frlnd(ns)%FBlndAccum2glc_g, value=0.0_r8, rc=rc) | ||
| if (chkerr(rc,__LINE__,u_FILE_u)) return | ||
| end do |
There was a problem hiding this comment.
This call to fldbun_reset was duplicated below, so I removed this unnecessary call.
|
I have run more extensive testing (noted in my edited top-level comment) and given the diffs a final, quick once-over myself. So this is now (finally!) ready for review. |
2 participants
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:
Some pieces are not yet implemented:
Depends on ESCOMP/CESM_share#82
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) 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.