Skip to content

update diag_manager documentation - #1869

Open
rem1776 wants to merge 21 commits into
NOAA-GFDL:mainfrom
rem1776:diag-docz
Open

update diag_manager documentation#1869
rem1776 wants to merge 21 commits into
NOAA-GFDL:mainfrom
rem1776:diag-docz

Conversation

@rem1776

@rem1776 rem1776 commented May 6, 2026

Copy link
Copy Markdown
Contributor

Description
Updates the diag_manager documentation.

I tried to incorporate more information about which code is rewritten vs legacy code.

How Has This Been Tested?
n/a

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules
  • New check tests, if applicable, are included
  • make distcheck passes

Comment thread diag_manager/diag_manager.F90 Outdated
Comment thread diag_manager/diag_manager.F90 Outdated
Comment thread diag_manager/fms_diag_axis_object.F90 Outdated
Comment thread diag_manager/fms_diag_axis_object.F90 Outdated
Comment thread diag_manager/fms_diag_axis_object.F90 Outdated
Comment thread diag_manager/fms_diag_axis_object.F90 Outdated
Comment thread diag_manager/fms_diag_axis_object.F90 Outdated
Comment thread diag_manager/README.md
Comment thread diag_manager/README.md Outdated
Comment thread diag_manager/README.md
Comment thread diag_manager/include/fms_diag_reduction_methods.inc Outdated
Comment thread diag_manager/include/fms_diag_reduction_methods.inc Outdated
@rem1776
rem1776 marked this pull request as ready for review May 19, 2026 16:33
@mlee03
mlee03 requested a review from Copilot June 3, 2026 18:01

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR primarily improves documentation and Doxygen grouping across the legacy/modern diag_manager code, adds dedicated namelist documentation, and makes a small error-message correction for consistency.

Changes:

  • Expanded/clarified module headers and added “legacy vs modern diag_manager” notes across multiple Fortran modules.
  • Added new diag_manager_nml Markdown documentation.
  • Minor message/documentation cleanups (e.g., updated fatal error prefix for fms_get_domain2d).

Reviewed changes

Copilot reviewed 18 out of 19 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
diag_manager/fms_diag_yaml.F90 Updated module brief to describe YAML object + read routines.
diag_manager/fms_diag_reduction_methods.F90 Expanded module documentation; added debug-flag instructions.
diag_manager/fms_diag_output_buffer.F90 Added Doxygen group annotations and clarified module brief.
diag_manager/fms_diag_outfield.F90 Marked as legacy module and pointed to modern equivalents.
diag_manager/fms_diag_object.F90 Corrected fatal error message to the right module/function name.
diag_manager/fms_diag_input_buffer.F90 Clarified purpose (OpenMP/threaded send_data buffering).
diag_manager/fms_diag_file_object.F90 Improved Doxygen docs for modern file-object module and type fields.
diag_manager/fms_diag_fieldbuff_update.F90 Marked as legacy module and pointed to modern equivalents.
diag_manager/fms_diag_field_object.F90 Added richer Doxygen groups/docs for modern field object module/type.
diag_manager/fms_diag_bbox.F90 Minor documentation/formatting cleanup in bbox module.
diag_manager/fms_diag_axis_object.F90 Significantly expanded axis/domain documentation and comments.
diag_manager/diag_util.F90 Added legacy note; removed obsolete commented “FUTURE” block.
diag_manager/diag_table.F90 Added legacy note pointing to modern YAML/object path.
diag_manager/diag_output.F90 Added legacy note pointing to modern I/O/object path.
diag_manager/diag_manager_nml.md New: Markdown documentation for diag_manager_nml variables.
diag_manager/diag_manager.F90 Expanded top-level module documentation and removed embedded namelist block.
diag_manager/diag_grid.F90 Added legacy note with a callout that one routine is still used by modern path.
diag_manager/diag_axis.F90 Added legacy note explaining modern routing behavior.
diag_manager/README.md Major expansion/restructure describing rewrite and YAML examples.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 263 to 265
!> @addtogroup fms_diag_yaml_mod
!> @{
contains
Comment on lines 69 to 74
public :: fmsDiagFileContainer_type
public :: fmsDiagFile_type, fms_diag_files_object_init, fms_diag_files_object_initialized
!> @}


logical :: fms_diag_files_object_initialized = .false.
Comment on lines +20 to +24
!! @brief fms_diag_reduction_methods_mod contains routines that are used to perform diagnostic reduction methods,
!! such as max/min, average, rms, etc. These routines are called by the send_data routines in the diag_manager.
!!
!! To debug the reduction methods, compile with `-DFMS_DIAG_DEBUG_REDUCT=.true.` and the reduction routines will print
!! out the values of the buffer and field data at each time step.
Comment on lines +26 to +28
!! @note This file is part of the legacy diag_manager.
!! If use_modern_diag is enabled, this module will be unused, as all IO will be handled
!! by the fms_diag_object.F90 module and it's helpers.
Comment thread diag_manager/README.md
Comment on lines 60 to 62
The modern diag manager uses a YAML format instead of the legacy ascii table. A description of the YAML diag table can
be found [here](diag_yaml_format.md). A formal specification, in the form of a JSON schema, can be found in the
[gfdl_msd_schemas](https://github.com/NOAA-GFDL/gfdl_msd_schemas) repository on Github.
Comment thread diag_manager/README.md
Comment on lines +79 to +82
varlist:
- var_name: var1
output_name: variable_one
reduction: average
Comment thread diag_manager/diag_grid.F90 Outdated
Comment thread diag_manager/fms_diag_reduction_methods.F90 Outdated
Comment thread diag_manager/fms_diag_input_buffer.F90 Outdated
Comment thread diag_manager/diag_manager_nml.md Outdated
Comment thread diag_manager/diag_manager_nml.md Outdated
Comment on lines +98 to +101
### `use_mpp_io`
- Type: `LOGICAL`
- Default: `.FALSE.`
- Description: Selects the I/O backend: true uses `mpp_io`, false uses `fms2_io` (recommended).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is not going to be an option after resolving #1881

Comment thread diag_manager/diag_manager_nml.md Outdated
Comment thread diag_manager/diag_manager_nml.md Outdated
Comment thread diag_manager/diag_manager_nml.md Outdated
Comment thread diag_manager/diag_manager_nml.md Outdated
Comment thread diag_manager/diag_manager_nml.md Outdated
Comment thread diag_manager/diag_manager.F90 Outdated
!! @brief diag_manager_mod is a set of simple calls for parallel diagnostics
!! on distributed systems. It is geared toward the writing of data in netCDF
!! format. See @ref diag_manager for diag table information.
!! format. The diag_manager differs from the fms2_io module in that it reads in

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Below is not perfect, but perhaps better organized than what's here?

Diag_manager consists of tools for handling diagnostic variables in parallel on distributed systems. It uses fms2_io under the hood to write the data in NetCDF format.

Diag_manager uses the diag_table to read in all user-defined diagnostic specifications such as [fill me in with stuff like file name, write frequencies,]. This module also handles 'reduction' operations such as taking the averaging or taking the min/max of the data. [link to all reduction methods]. For example, users can send_data for a field multiple times for a timestep and the output will be 'reduced' as specified in the diag_table.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I added this but tweaked it a bit.

Comment thread diag_manager/diag_manager.F90
Comment thread diag_manager/diag_manager.F90 Outdated
Comment thread diag_manager/diag_manager_nml.md Outdated
Comment thread diag_manager/diag_manager_nml.md Outdated
Comment thread diag_manager/README.md Outdated
Comment thread diag_manager/README.md Outdated
Comment thread diag_manager/README.md Outdated
Comment thread diag_manager/README.md
@@ -80,7 +221,7 @@ In some cases, the old diag manager was adding `sub0X` to the dimension names wh
#### C. Corner and center diagnostics:
In the old diag manager, if mixing variables that are corner variables, such as velocities={uo,vo,umo,vmo} and center variables, such as tracers={thetao,so,volcello} you sometimes ended up with a different number of variables per file. The extra files had duplicate data for the corner velocities because the two PEs shared the point at the edge. This happened with some grid/layouts/masks/subregion combinations and it caused problems with the combiner. The new diag manager will not have this problem.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is not clear..

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don't know if i can explain this much better than Uriel did. I think this is only really important to users of the old diag_manager and they should understand this so maybe its fine to leave as-is?

Comment thread diag_manager/fms_diag_field_object.F90 Outdated
Comment thread diag_manager/diag_manager_nml.md Outdated
### `use_clock_average`
- Type: `LOGICAL`
- Default: `.FALSE.`
Description: Controls how averaging windows are defined. When true, averaging of variables is done based on the clock. For example, if enabled and you start at day 1 hour 5, a 1 day freqency will only account for the rest of the hours in that day, so 19 hours total. Normally, the averaging would be done over the full 24 hour period regardless if it goes into the next day.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's not clear what clock is referring to.. a FMSTimeType?

Comment thread diag_manager/diag_table.F90 Outdated
!! @author Seth Underwood
!!
!! @note This module is part of the legacy diag_manager. If use_modern_diag is enabled, this module will be unused,
!! as all parsing and data storage will be handled by the fms_diag_object.F90 module and it's helpers.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

another case of it's --> its

Comment thread diag_manager/fms_diag_axis_object.F90 Outdated
!! This type extends the diagDomain_t base type and is used when an axis is
!! associated with a 1D domain (typically a vertical or time axis).
!! The 1D domain provides information about how the axis is partitioned across
!! MPI processes along a single dimension.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

remove "along a single dimension"

!!
!! This type extends the diagDomain_t base type and is used when an axis is
!! associated with an unstructured (irregular) grid domain. Unstructured grids are commonly
!! used in models with non-uniform spatial decomposition, such as icosahedral grids.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is this true?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants