Skip to content

Simplify diag_table templates and introduce a stream-based writer - #341

Merged
alperaltuntas merged 2 commits into
ESCOMP:mainfrom
alperaltuntas:diag_table_stream_names
Sep 4, 2026
Merged

Simplify diag_table templates and introduce a stream-based writer#341
alperaltuntas merged 2 commits into
ESCOMP:mainfrom
alperaltuntas:diag_table_stream_names

Conversation

@alperaltuntas

@alperaltuntas alperaltuntas commented Aug 19, 2026

Copy link
Copy Markdown
Member

This PR restructures how the MOM6 diag_table is generated, so as to simplify the template files and to prepare for user-customizable diagnostic streams via user_nl_mom (which will come in a follow-up PR). The main changes:

  • Introduces diag_table_streams.py, a new module containing the stream model (Stream, Field, FieldGroup) and the diag_table writer. A stream is one output file: an entry in the file list of the diag_table together with the fields written to that file.
  • Rewrites FType_diag_table.write to build streams from the reduced template and delegate the writing to the new module. Template entries that resolve to the same file name are now merged, with consistency checks on their settings and fields.
  • Simplifies diag_table.yaml: common settings now live in a new FileDefaults section, file entries are labeled directly with their stream names (e.g., h.native, h.sfc), and the date part of file names is no longer spelled out in a suffix setting but derived from new_file_freq_units.
  • Updates MARBL_diags_to_diag_table.py accordingly: entries now provide a name instead of a suffix, and packing is a stream setting rather than a per-fields-block setting.
  • Adds more thorough validation with (hopefully) more helpful error messages: unknown settings, invalid reduction methods and regional sections, duplicate fields, and conflicting entries are all reported along with the offending template entry and stream name.

No answer changes. The generated diag_table is identical to before (verified by diffing old vs. new diag_tables over all combinations of OCN_DIAG_MODE, TEST, OCN_GRID, OCN_DIAG_SECTIONS, USE_CFC_CAP, DO_GEOTHERMAL, and MARBL configurations), except for three intended differences:

  1. section streams now use "mean" instead of the equivalent ".true." reduction spelling,
  2. TEST-mode file names of h.visc and h.bgc.native now include the day (%2dy), which fixes a filename collision caused by daily rollover combined with a monthly name template,
  3. MARBL entries that resolve to the same file (e.g., the 2D and 3D-native "low" streams) are merged into a single file entry instead of producing duplicate file list lines and duplicate geolat/geolon fields.

Note: this changes the format of diag_table_MARBL.json, so existing copies of that file in SourceMods need to be updated (suffix -> name, and packing moved out of the fields block). The build fails with an explanatory error message otherwise.

Testing: aux_mom.derecho (namelists only): all PASS
Status: b4b

@alperaltuntas @claude
Name diag_table streams after their output files
c078e72 
The label of each entry in the Files section of `diag_table.yaml` is now
the stream name, i.e., the part of the output file name that follows the
case name and `mom6`: `sigma2_hist` becomes `h.rho2`, `hist_z_space`
becomes `h.z`, `visc_and_diff_daily_avg` becomes `h.visc`, and so on.
Users see these names on disk and in the archive, so these are also the
names they will refer to once we add the ability to modify the diag_table
via `user_nl_mom`.

The date template of the file name is no longer written by hand: it is
derived from how often a new file is started, since a name that lacks
that time resolution gets overwritten by the next file of the same
stream. The `suffix` entry is therefore gone. Entries that need a
configuration dependent name provide an optional guarded `name` entry
instead (only the MARBL entries do, since they rename their files in
spinup runs.)

Settings shared by all files moved to a new `FileDefaults` section, and
`new_file_freq` now defaults to one file per unit of the output
frequency. Together, these bring `diag_table.yaml` from 769 to 404 lines
without losing expressiveness: the base name of every stream was already
the same in all 32 combinations of `TEST`, `OCN_DIAG_MODE`, `OCN_GRID`,
and `OCN_DIAG_SECTIONS`.

`FType_diag_table` is restructured around the stream model that now lives
in `diag_table_streams`, which has no CIME dependency so that it can be
tested on its own. Generating the table in terms of streams rather than
template entries also fixes a few latent problems:

 - Two entries that resolve to the same file are now merged into one
   stream. MARBL's `low` and `low_native_z` do resolve to the same file
   when `MARBL_HIST_VERT_GRID` is native or both, and used to emit a
   duplicate file entry in Section-1 along with duplicate `geolat` and
   `geolon` fields. (Its `medium` entries have the same problem, worked
   around in `MARBL_diags_to_diag_table` by combining them.)
 - A fields or lists block whose guards are all false reduces to None,
   which is now skipped rather than passed to len() or max(). Only the
   ordering of the blocks kept this from being reachable.
 - The check for a field listed more than once in the same file only ever
   looked within a single fields block.
 - Whether `new_file_freq_units` gets written was decided by testing for
   `time_axis_units`.
 - Stream settings are now validated, and a malformed template gets an
   error naming the entry it came from rather than a TypeError or a
   KeyError from deeper down.

Also drops an unused import in `FType_input_nml`.

status: no answer changes. MOM6 reads the table no differently, and
diagnostic contents are unchanged in all the configurations compared,
with these exceptions: `h.visc` in tests and, with
`MARBL_HIST_VERT_GRID` native or both, `h.bgc.native` in tests both
started a new file every day while their names only resolved to the
month, so they used to overwrite themselves and now get a daily date
template. The section files say "mean" where they used to say ".true.";
FMS maps both to the same time average (see `init_output_field` in
`diag_util.F90`). Column padding is now driven by what is actually
written to the table.

testing: rendered the diag_table for 96 combinations of `TEST`,
`OCN_DIAG_MODE`, `OCN_GRID`, `OCN_DIAG_SECTIONS`, and
`MARBL_HIST_VERT_GRID`, and compared each against the current output.
The scripts in `tests/` pass. aux_mom has not been run yet.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@alperaltuntas alperaltuntas changed the title Name diag_table streams after their output files Simplify diag_table templates and introduce a stream-based writer Aug 19, 2026
@alperaltuntas

alperaltuntas commented Aug 26, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

@mnlevy1981 Friendly reminder that this PR is waiting on review. A quick look at the changes in MARBL_diags_to_diag_table.py would be enough, though feel free to do a deeper review if you'd prefer.

Once this is merged, I'll open a follow-up PR that adds support for editing diag_table via user_nl_mom. I'll then use that feature to switch the DIMCS[L] tests from G_JRA to GW_JRA. Those currently fail when waves are enabled, due to a known diagnostics issue.

mnlevy1981
mnlevy1981 approved these changes Aug 31, 2026
View reviewed changes

@mnlevy1981 mnlevy1981 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I don't see any problem with the MARBL portion of the diag table, but I do see two types of minor differences when comparing diag_table.unresolved between two cases (one generated using the mi_260715 tag of MOM_interface and the other using this branch):

  1. Some formatting differences in the h.sfc stream (I ran diff -u <(sort ${BASE_CASE}/Buildconf/momconf/diag_table.unresolved) <(sort ${NEW_CASE}/Buildconf/momconf/diag_table.unresolved))
@@ -291,11 +291,11 @@
 "ocean_model", "Jint_100m_SiO3","Jint_100m_SiO3","${CASE}.mom6.h.bgc.native%4yr-%2mo", "all", "mean", "none", 2
 "ocean_model", "KE",           "KE",           "${CASE}.mom6.h.native%4yr-%2mo", "all", "mean", "none", 2
 "ocean_model", "KPP_OBLdepth", "oml",          "${CASE}.mom6.h.native%4yr-%2mo", "all", "mean", "none", 2
-"ocean_model", "KPP_OBLdepth", "oml",          "${CASE}.mom6.h.sfc%4yr-%2mo", "all", "mean", "none", 2
+"ocean_model", "KPP_OBLdepth","oml",         "${CASE}.mom6.h.sfc%4yr-%2mo", "all", "mean", "none", 2
 "ocean_model", "KPP_OBLdepth", "oml_max",      "${CASE}.mom6.h.native%4yr-%2mo", "all", "max", "none", 2
-"ocean_model", "KPP_OBLdepth", "oml_max",      "${CASE}.mom6.h.sfc%4yr-%2mo", "all", "max", "none", 2
+"ocean_model", "KPP_OBLdepth","oml_max",     "${CASE}.mom6.h.sfc%4yr-%2mo", "all", "max", "none", 2
 "ocean_model", "KPP_OBLdepth", "oml_min",      "${CASE}.mom6.h.native%4yr-%2mo", "all", "min", "none", 2
-"ocean_model", "KPP_OBLdepth", "oml_min",      "${CASE}.mom6.h.sfc%4yr-%2mo", "all", "min", "none", 2
+"ocean_model", "KPP_OBLdepth","oml_min",     "${CASE}.mom6.h.sfc%4yr-%2mo", "all", "min", "none", 2
 "ocean_model", "Kv_u",         "Kv_u",         "${CASE}.mom6.h.native%4yr-%2mo", "all", "mean", "none", 2
 "ocean_model", "Kv_v",         "Kv_v",         "${CASE}.mom6.h.native%4yr-%2mo", "all", "mean", "none", 2
 "ocean_model", "mass_wt",      "mass_wt",      "${CASE}.mom6.h.native%4yr-%2mo", "all", "mean", "none", 2
@@ -321,11 +321,11 @@
 "ocean_model", "microzoo_loss_zint_150m","microzoo_loss_zint_150m","${CASE}.mom6.h.bgc.daily%4yr-%2mo", "all", "mean", "none", 2
 "ocean_model", "microzoo_loss_zint","microzoo_loss_zint","${CASE}.mom6.h.bgc.native%4yr-%2mo", "all", "mean", "none", 2
 "ocean_model", "mlotst",       "mlotst",       "${CASE}.mom6.h.native%4yr-%2mo", "all", "mean", "none", 2
-"ocean_model", "mlotst",       "mlotst",       "${CASE}.mom6.h.sfc%4yr-%2mo", "all", "mean", "none", 2
+"ocean_model", "mlotst",      "mlotst",      "${CASE}.mom6.h.sfc%4yr-%2mo", "all", "mean", "none", 2
 "ocean_model", "mlotst",       "mlotst_max",   "${CASE}.mom6.h.native%4yr-%2mo", "all", "max", "none", 2
-"ocean_model", "mlotst",       "mlotst_max",   "${CASE}.mom6.h.sfc%4yr-%2mo", "all", "max", "none", 2
+"ocean_model", "mlotst",      "mlotst_max",  "${CASE}.mom6.h.sfc%4yr-%2mo", "all", "max", "none", 2
 "ocean_model", "mlotst",       "mlotst_min",   "${CASE}.mom6.h.native%4yr-%2mo", "all", "min", "none", 2
-"ocean_model", "mlotst",       "mlotst_min",   "${CASE}.mom6.h.sfc%4yr-%2mo", "all", "min", "none", 2
+"ocean_model", "mlotst",      "mlotst_min",  "${CASE}.mom6.h.sfc%4yr-%2mo", "all", "min", "none", 2
 "ocean_model", "mlotstsq",     "mlotstsq",     "${CASE}.mom6.h.native%4yr-%2mo", "all", "mean", "none", 2
 "ocean_model", "NHx_SURFACE_EMIS","NHx_SURFACE_EMIS","${CASE}.mom6.h.bgc.native%4yr-%2mo", "all", "mean", "none", 2
 "ocean_model", "NHy_FLUX",     "NHy_FLUX",     "${CASE}.mom6.h.bgc.native%4yr-%2mo", "all", "mean", "none", 2
  1. It looks like reduction_method for the regional transports was changed from .true. to mean, though the FMS documentation states that all of .TRUE., average, avg, mean are treated the same
@@ -710,28 +710,28 @@
 "ocean_model_z", "SiO2_FLUX_IN", "SiO2_FLUX_IN", "${CASE}.mom6.h.bgc.z%4yr-%2mo", "all", "mean", "none", 2
 "ocean_model_z", "SiO2_PROD",    "SiO2_PROD",    "${CASE}.mom6.h.bgc.z%4yr-%2mo", "all", "mean", "none", 2
 "ocean_model_z", "SiO3",         "SiO3",         "${CASE}.mom6.h.bgc.z%4yr-%2mo", "all", "mean", "none", 2
-"ocean_model_z", "so",      "so",      "${CASE}.mom6.h.Agulhas_Section%4yr-%2mo", "all", ".true.", "20.1 20.1 -69.8 -34.6 -1 -1", 2
-"ocean_model_z", "so",      "so",      "${CASE}.mom6.h.Barents_Opening%4yr-%2mo", "all", ".true.", "-5.75 19.0  78.93 78.93 -1 -1", 2
-"ocean_model_z", "so",      "so",      "${CASE}.mom6.h.Bering_Strait%4yr-%2mo", "all", ".true.", "-174.5 -172.3 66.6 66.6 -1 -1", 2
+"ocean_model_z", "so",      "so",      "${CASE}.mom6.h.Agulhas_Section%4yr-%2mo", "all", "mean", "20.1 20.1 -69.8 -34.6 -1 -1", 2
+"ocean_model_z", "so",      "so",      "${CASE}.mom6.h.Barents_Opening%4yr-%2mo", "all", "mean", "-5.75 19.0  78.93 78.93 -1 -1", 2
+"ocean_model_z", "so",      "so",      "${CASE}.mom6.h.Bering_Strait%4yr-%2mo", "all", "mean", "-174.5 -172.3 66.6 66.6 -1 -1", 2

I suspect [1] was an inadvertent oversight somewhere that should be fixed; I don't know if [2] was an intentional choice for consistency or to bring our diag_table more in-line with other modeling centers, or if we want to revert back to the previous reduction_method. I'm happy with how this looks, though, no one else is concerned about these trivial differences.

This new schema makes MARBL_diags_to_diag_table.py more readable, which I appreciate.

@alperaltuntas

alperaltuntas commented Sep 4, 2026

Copy link
Copy Markdown
Member Author

Thanks @mnlevy1981 . [1] is now fixed. I reverted to the previous column width formula. As for [2], yes it's intentional. Previously, we were using both .true. and mean arbitrarily. Now we are using mean uniformly, which is also more descriptive.

@alperaltuntas
alperaltuntas merged commit 6bbb9ba into ESCOMP:main Sep 4, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mnlevy1981 mnlevy1981 mnlevy1981 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@alperaltuntas @mnlevy1981