Skip to content

Add diagnostic metadata property to constituents object; use existing diagnostic name metadata attribute #695

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mkavulich merged 6 commits into from
Jan 15, 2026
Merged

Add diagnostic metadata property to constituents object; use existing diagnostic name metadata attribute #695

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
aab4a9e
add diagnostic name property to constituents properties object and us…
peverwhee Nov 18, 2025
9d8fe6a
fix doctest
peverwhee Nov 18, 2025
549e2ac
address review comment
peverwhee Dec 15, 2025
2d189fc
Merge remote-tracking branch 'NCAR/develop' into diagnostic-name
peverwhee Dec 15, 2025
e73a7bc
add additional doctests to new diagnostic name conversion
peverwhee Jan 2, 2026
4601a0e
Merge remote-tracking branch 'NCAR/develop' into diagnostic-name
peverwhee Jan 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion scripts/ccpp_datafile.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -890,7 +890,7 @@ def _new_var_entry(parent, var, full_entry=True):
>>> var = Var({'local_name' : 'foo', 'standard_name' : 'hi_mom', 'units' : 'm s-1', 'dimensions' : '(horizontal_loop_extent)', 'type' : 'real', 'intent' : 'in'}, ParseSource('vname', 'DDT', ParseContext()), _MVAR_DUMMY_RUN_ENV)
>>> _new_var_entry(parent, var)
>>> table_entry_pretty_print(parent, 0)
'<variables>\\n <var name=hi_mom intent=in local_name=foo active=.true. kind=kind_phys persistence=timestep type=real units=m s-1>\\n <dimensions>\\n horizontal_loop_extent\\n </dimensions>\\n <source_type>\\n ddt\\n </source_type>\\n <source_name>\\n vname\\n </source_name>\\n </var>\\n</variables>\\n'
'<variables>\\n <var name=hi_mom intent=in local_name=foo active=.true. diagnostic_name=foo kind=kind_phys persistence=timestep type=real units=m s-1>\\n <dimensions>\\n horizontal_loop_extent\\n </dimensions>\\n <source_type>\\n ddt\\n </source_type>\\n <source_name>\\n vname\\n </source_name>\\n </var>\\n</variables>\\n'

>>> parent = ET.fromstring('<variables></variables>')
>>> _new_var_entry(parent, var, full_entry=False)
Expand Down
3 changes: 2 additions & 1 deletion scripts/constituents.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,7 @@ def write_constituent_routines(self, outfile, indent, suite_name, err_vars):
# end if
outfile.write("index = index + 1", indent+1)
long_name = var.get_prop_value('long_name')
diag_name = var.get_prop_value('diagnostic_name')
units = var.get_prop_value('units')
dims = var.get_dim_stdnames()
default_value = var.get_prop_value('default_value')
Expand All @@ -311,7 +312,7 @@ def write_constituent_routines(self, outfile, indent, suite_name, err_vars):
vertical_dim = ''
# end if
advect_str = self.TF_string(var.get_prop_value('advected'))
init_args = [f'{std_name=}', f'{long_name=}',
init_args = [f'{std_name=}', f'{long_name=}', f'{diag_name=}',
f'{units=}', f'{vertical_dim=}',
f'advected={advect_str}',
f'errcode={errvar_names["ccpp_error_code"]}',
Expand Down
8 changes: 4 additions & 4 deletions scripts/metavar.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
from parse_tools import FORTRAN_CONDITIONAL_REGEX_WORDS, FORTRAN_CONDITIONAL_REGEX
from var_props import CCPP_LOOP_DIM_SUBSTS, VariableProperty, VarCompatObj
from var_props import find_horizontal_dimension, find_vertical_dimension
from var_props import standard_name_to_long_name, default_kind_val
from var_props import standard_name_to_long_name, local_name_to_diag_name, default_kind_val

##############################################################################

Expand Down Expand Up @@ -174,6 +174,9 @@ class Var:
check_fn_in=check_cf_standard_name),
VariableProperty('long_name', str, optional_in=True,
default_fn_in=standard_name_to_long_name),
VariableProperty('diagnostic_name', str, optional_in=True,
default_fn_in=local_name_to_diag_name,
check_fn_in=check_diagnostic_id),
VariableProperty('units', str,
check_fn_in=check_units),
VariableProperty('dimensions', list,
Expand All @@ -189,9 +192,6 @@ class Var:
optional_in=True, default_in=False),
VariableProperty('allocatable', bool,
optional_in=True, default_in=False),
VariableProperty('diagnostic_name', str,
optional_in=True, default_in='',
check_fn_in=check_diagnostic_id),
VariableProperty('diagnostic_name_fixed', str,
optional_in=True, default_in='',
check_fn_in=check_diagnostic_fixed),
Expand Down
38 changes: 38 additions & 0 deletions scripts/var_props.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,44 @@ def find_vertical_dimension(dims):
# end for
return (var_vdim, vindex)

########################################################################
def local_name_to_diag_name(prop_dict, context=None):
########################################################################
"""
Translate a local_name to its default diagnostic name.
Currently, this is just equal to the local name. If no local name
exists in the property dictionary, a truncation of the standard
name is used. (256 characters = max length of NetCDF variable name)
>>> local_name_to_diag_name({'local_name':'foo', 'standard_nam':'cloud_optical_depth'})
Copy link
Collaborator

@gold2718 gold2718 Dec 30, 2025

Choose a reason for hiding this comment

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

Is the missing letter on purpose?

Suggested change
>>> local_name_to_diag_name({'local_name':'foo', 'standard_nam':'cloud_optical_depth'})
>>> local_name_to_diag_name({'local_name':'foo', 'standard_name':'cloud_optical_depth'})

Copy link
Collaborator Author

@peverwhee peverwhee Jan 2, 2026

Choose a reason for hiding this comment

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

nope! fixed!

'foo'
>>> local_name_to_diag_name({'standard_name':'cloud_optical_depth_layers_from_0p55mu_to_0p99mu'})
'cloud_optical_depth_layers_from_0p55mu_to_0p99mu'
>>> local_name_to_diag_name({'units':'km'}) #doctest: +ELLIPSIS
Traceback (most recent call last):
...
parse_source.CCPPError: No standard name or local name to convert to diagnostic name
"""
Copy link
Collaborator

@gold2718 gold2718 Dec 30, 2025

Choose a reason for hiding this comment

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

I might add a couple of tests for more complete code coverage:

Suggested change
"""
>>> local_name_to_diag_name({'local_name':'', 'standard_name':'cloud_optical_depth'}) #doctest: +ELLIPSIS
Traceback (most recent call last):
...
parse_source.CCPPError: No standard name or local name to convert to diagnostic name
>>> local_name_to_diag_name({'standard_name':''}) #doctest: +ELLIPSIS
Traceback (most recent call last):
...
parse_source.CCPPError: No standard name or local name to convert to diagnostic name
"""

Copy link
Collaborator Author

@peverwhee peverwhee Jan 2, 2026

Choose a reason for hiding this comment

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

thanks for the extra doctests! added!

diag_name = None
if 'local_name' in prop_dict:
locname = prop_dict['local_name']
if locname:
diag_name = prop_dict['local_name']
# end if
elif 'standard_name' in prop_dict:
stdname = prop_dict['standard_name']
if stdname:
maxlen = 256
Copy link
Collaborator

@climbfuji climbfuji Jan 5, 2026

Choose a reason for hiding this comment

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

I wonder if hardcoding this value in this particular place is a good idea. I would be surprised if we didn't have a parameter for the maximum length of a standard name somewhere else already.

Copy link
Collaborator Author

@peverwhee peverwhee Jan 8, 2026

Choose a reason for hiding this comment

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

@climbfuji it looks like we're not enforcing a maximum standard name length anywhere! At least that I can find.

I'm enforcing it here because NetCDF has a maximum length for variable names.

Copy link
Collaborator

@climbfuji climbfuji Jan 8, 2026

Choose a reason for hiding this comment

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

Is that a good opportunity then to define that maxlen parameter in the appropriate place, or do you want to keep it here for now/simplicity?

Copy link
Collaborator Author

@peverwhee peverwhee Jan 8, 2026

Choose a reason for hiding this comment

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

I'm not sure! I don't know if we want a hard limit on the standard name length?

Copy link
Collaborator

@climbfuji climbfuji Jan 9, 2026

Choose a reason for hiding this comment

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

It does sound like a limitation if we say standard names cannot be longer than the maximum string length in netcdf, but it makes total sense to me. I would question the usefulness of standard names longer than 256, maybe even 128 characters.

Copy link
Collaborator Author

@peverwhee peverwhee Jan 9, 2026

Choose a reason for hiding this comment

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

Sounds like something we should discuss at a framework meeting - I'll make an issue.

Copy link
Collaborator

@climbfuji climbfuji Jan 9, 2026

Choose a reason for hiding this comment

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

Thanks! We can come back and clean this up later if needed. Good to merge I think.

diag_name = stdname[:maxlen] if len(stdname) > maxlen else stdname
Copy link
Collaborator

@gold2718 gold2718 Nov 23, 2025

Choose a reason for hiding this comment

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

Is this necessary?
From https://docs.python.org/3/library/stdtypes.html#text-sequence-type-str
"The slice of s from i to j is defined as the sequence of items with index k such that i <= k < j. If i or j is greater than len(s), use len(s)."

Suggested change
maxlen = 256
diag_name = stdname[:maxlen] if len(stdname) > maxlen else stdname
maxlen = 256
diag_name = stdname[:maxlen]

Copy link
Collaborator

@mkavulich mkavulich Dec 4, 2025

Choose a reason for hiding this comment

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

@peverwhee Do you have an answer to this question?

Copy link
Collaborator Author

@peverwhee peverwhee Dec 15, 2025

Choose a reason for hiding this comment

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

yes sorry for the delay! I've pushed up @gold2718 's suggested change

# end if
# end if

if not diag_name:
emsg = 'No standard name or local name to convert to diagnostic name'
raise CCPPError(emsg)
# end if

return diag_name

########################################################################
def standard_name_to_long_name(prop_dict, context=None):
########################################################################
Expand Down
57 changes: 54 additions & 3 deletions src/ccpp_constituent_prop_mod.F90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ module ccpp_constituent_prop_mod
! for a constituent species and provides interfaces to access that data.
character(len=:), private, allocatable :: var_std_name
character(len=:), private, allocatable :: var_long_name
character(len=:), private, allocatable :: var_diag_name
character(len=:), private, allocatable :: var_units
character(len=:), private, allocatable :: vert_dim
integer, private :: const_ind = int_unassigned
Expand All @@ -60,6 +61,7 @@ module ccpp_constituent_prop_mod
procedure :: is_instantiated => ccp_is_instantiated
procedure :: standard_name => ccp_get_standard_name
procedure :: long_name => ccp_get_long_name
procedure :: diagnostic_name => ccp_get_diagnostic_name
procedure :: units => ccp_get_units
procedure :: is_layer_var => ccp_is_layer_var
procedure :: is_interface_var => ccp_is_interface_var
Expand Down Expand Up @@ -103,6 +105,7 @@ module ccpp_constituent_prop_mod
! Informational methods
procedure :: standard_name => ccpt_get_standard_name
procedure :: long_name => ccpt_get_long_name
procedure :: diagnostic_name => ccpt_get_diagnostic_name
procedure :: units => ccpt_get_units
procedure :: is_layer_var => ccpt_is_layer_var
procedure :: is_interface_var => ccpt_is_interface_var
Expand Down Expand Up @@ -219,6 +222,7 @@ subroutine copyConstituent(outConst, inConst)

outConst%var_std_name = inConst%var_std_name
outConst%var_long_name = inConst%var_long_name
outConst%var_diag_name = inConst%var_diag_name
outConst%vert_dim = inConst%vert_dim
outConst%const_ind = inConst%const_ind
outConst%advected = inConst%advected
Expand Down Expand Up @@ -373,15 +377,16 @@ end function ccp_is_instantiated

!#######################################################################

subroutine ccp_instantiate(this, std_name, long_name, units, vertical_dim, &
advected, default_value, min_value, molar_mass, water_species, &
subroutine ccp_instantiate(this, std_name, long_name, diag_name, units, &
vertical_dim, advected, default_value, min_value, molar_mass, water_species, &
mixing_ratio_type, errcode, errmsg)
! Initialize all fields in <this>

! Dummy arguments
class(ccpp_constituent_properties_t), intent(inout) :: this
character(len=*), intent(in) :: std_name
character(len=*), intent(in) :: long_name
character(len=*), intent(in) :: diag_name
character(len=*), intent(in) :: units
character(len=*), intent(in) :: vertical_dim
logical, optional, intent(in) :: advected
Expand All @@ -404,6 +409,7 @@ subroutine ccp_instantiate(this, std_name, long_name, units, vertical_dim, &
end if
if (errcode == 0) then
this%var_long_name = trim(long_name)
this%var_diag_name = trim(diag_name)
this%var_units = trim(units)
this%vert_dim = trim(vertical_dim)
if (present(advected)) then
Expand Down Expand Up @@ -479,6 +485,9 @@ subroutine ccp_deallocate(this)
if (allocated(this%var_long_name)) then
deallocate(this%var_long_name)
end if
if (allocated(this%var_diag_name)) then
deallocate(this%var_diag_name)
end if
if (allocated(this%vert_dim)) then
deallocate(this%vert_dim)
end if
Expand Down Expand Up @@ -530,6 +539,25 @@ end subroutine ccp_get_long_name

!#######################################################################

subroutine ccp_get_diagnostic_name(this, diag_name, errcode, errmsg)
! Return this constituent's diagnostic name

! Dummy arguments
class(ccpp_constituent_properties_t), intent(in) :: this
character(len=*), intent(out) :: diag_name
integer, optional, intent(out) :: errcode
character(len=*), optional, intent(out) :: errmsg

if (this%is_instantiated(errcode, errmsg)) then
diag_name = this%var_diag_name
else
diag_name = ''
end if

end subroutine ccp_get_diagnostic_name

!#######################################################################

subroutine ccp_get_units(this, units, errcode, errmsg)
! Return this constituent's units

Expand Down Expand Up @@ -763,6 +791,7 @@ subroutine ccp_is_equivalent(this, oconst, equiv, errcode, errmsg)
oconst%is_instantiated(errcode, errmsg)) then
equiv = (trim(this%var_std_name) == trim(oconst%var_std_name)) .and. &
(trim(this%var_long_name) == trim(oconst%var_long_name)) .and. &
(trim(this%var_diag_name) == trim(oconst%var_diag_name)) .and. &
(trim(this%vert_dim) == trim(oconst%vert_dim)) .and. &
(trim(this%var_units) == trim(oconst%var_units)) .and. &
(this%advected .eqv. oconst%advected) .and. &
Expand Down Expand Up @@ -1007,7 +1036,6 @@ logical function ccp_is_match(this, comp_props) result(is_match)
! Local variable
logical :: val, comp_val
character(len=stdname_len) :: char_val, char_comp_val
logical :: check

! By default, every constituent is a match
is_match = .true.
Expand Down Expand Up @@ -1989,6 +2017,29 @@ end subroutine ccpt_get_long_name

!#######################################################################

subroutine ccpt_get_diagnostic_name(this, diag_name, errcode, errmsg)
! Return this constituent's diagnostic name

! Dummy arguments
class(ccpp_constituent_prop_ptr_t), intent(in) :: this
character(len=*), intent(out) :: diag_name
integer, optional, intent(out) :: errcode
character(len=*), optional, intent(out) :: errmsg
! Local variable
character(len=*), parameter :: subname = 'ccpt_get_diagnostic_name'

if (associated(this%prop)) then
call this%prop%diagnostic_name(diag_name, errcode, errmsg)
else
diag_name = ''
call append_errvars(1, ": invalid constituent pointer", &
subname, errcode=errcode, errmsg=errmsg)
end if

end subroutine ccpt_get_diagnostic_name

!#######################################################################

subroutine ccpt_get_units(this, units, errcode, errmsg)
! Return this constituent's units

Expand Down
10 changes: 5 additions & 5 deletions test/advection_test/cld_ice.F90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,13 @@ subroutine cld_ice_register(dyn_const_ice, errmsg, errcode)
return
end if
call dyn_const_ice(1)%instantiate(std_name='dyn_const1', long_name='dyn const1', &
units='kg kg-1', default_value=0._kind_phys, &
vertical_dim='vertical_layer_dimension', advected=.true., &
min_value=1000._kind_phys, water_species=.true., mixing_ratio_type='wet', &
diag_name='DYNCONST1', units='kg kg-1', default_value=0._kind_phys, &
vertical_dim='vertical_layer_dimension', advected=.true., &
min_value=1000._kind_phys, water_species=.true., mixing_ratio_type='wet', &
errcode=errcode, errmsg=errmsg)
call dyn_const_ice(2)%instantiate(std_name='dyn_const2_wrt_moist_air', long_name='dyn const2', &
units='kg kg-1', default_value=0._kind_phys, &
vertical_dim='vertical_layer_dimension', advected=.true., &
diag_name='DYNCONST2', units='kg kg-1', default_value=0._kind_phys, &
vertical_dim='vertical_layer_dimension', advected=.true., &
water_species=.false., errcode=errcode, errmsg=errmsg)

end subroutine cld_ice_register
Expand Down
2 changes: 1 addition & 1 deletion test/advection_test/cld_liq.F90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ subroutine cld_liq_register(dyn_const, errmsg, errflg)
return
end if
call dyn_const(1)%instantiate(std_name="dyn_const3_wrt_moist_air_and_condensed_water", long_name='dyn const3', &
units='kg kg-1', default_value=1._kind_phys, &
diag_name='DYNCONST3', units='kg kg-1', default_value=1._kind_phys, &
vertical_dim='vertical_layer_dimension', advected=.true., &
water_species=.true., mixing_ratio_type='dry', &
errcode=errflg, errmsg=errmsg)
Expand Down
1 change: 1 addition & 0 deletions test/advection_test/cld_liq.meta
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@
intent = in
[ cld_liq_array ]
standard_name = cloud_liquid_dry_mixing_ratio
diagnostic_name = CLDLIQ
advected = .true.
units = kg kg-1
dimensions = (horizontal_dimension, vertical_layer_dimension)
Expand Down
2 changes: 1 addition & 1 deletion test/advection_test/dlc_liq.F90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ subroutine dlc_liq_init(dyn_const, errmsg, errflg)
return
end if
call dyn_const(1)%instantiate(std_name="dyn_const3", long_name='dyn const3', &
units='kg kg-1', default_value=1._kind_phys, &
diag_name='DYNCONST3', units='kg kg-1', default_value=1._kind_phys, &
vertical_dim='vertical_layer_dimension', advected=.true., &
errcode=errflg, errmsg=errmsg)
call dyn_const(1)%standard_name(stdname, errcode=errflg, errmsg=errmsg)
Expand Down
Loading