Skip to content
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

Add support and testing for equivalent units #571

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

Add support and testing for equivalent units #571

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d4d89a0
Add support and testing for equivalent units
dustinswales Jun 14, 2024
a45179d
Merge branch 'develop' of https://github.com/NCAR/ccpp-framework into…
dustinswales Jan 2, 2025
0bfe13b
Return Var instead of None for equivalent units. Update tests.
dustinswales Jan 2, 2025
8b96744
Merge branch 'develop' of https://github.com/NCAR/ccpp-framework into…
dustinswales Jan 7, 2025
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
11 changes: 11 additions & 0 deletions scripts/conversion_tools/unit_conversion.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,3 +176,14 @@ def W_m_minus_2__to__erg_cm_minus_2_s_minus_1():
def erg_cm_minus_2_s_minus_1__to__W_m_minus_2():
"""Convert erg per square centimeter and second to Watt per square meter"""
return '1.0E-3{kind}*{var}'

##################
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't know what this is called in the DSM-5 but I really want a couple of more hash marks here :)
I know where to get some cheap :)

Copy link
Collaborator

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 work with prebuild (at least the way the PR is written now). The unit conversions above return

def erg_cm_minus_2_s_minus_1__to__W_m_minus_2():
    """Convert erg per square centimeter and second to Watt per square meter"""
    return '1.0E-3{kind}*{var}'

in other words, they contain the input variable in the result string. I think you need to make similar changes to the ccpp_prebuild logic (if None then skip the unit conversion altogether), or you may get an error in the Python code, or in the auto-generated Fortran code.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, I didn't add this to prebuild yet.
If we agree on this for Capgen, then I could add it in to Prebuild as part of this PR.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@gold2718 Nothing is cheap in Norway, not even hashes from what I hear :)
And now I know what DSM-5 is.
(I will add some more hashes in for prettiness)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nothing is cheap in Norway, not even hashes from what I hear :)

That's what a harrytur* is for.

*Harrytur: Norwegian slang for driving to Sweden to buy stuff that's cheaper there.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@dustinswales The easiest way (no changes at all in prebuild) would be to return just the input variable in the conversion string ({var}) for equivalent units, not None. If a compiler finds foo=foo in the fortran code, it is very likely going to optimize out that line anyway.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@climbfuji Done.
Although I think returning None and skipping writing the equivalence statement is the way to go to skip adding all of "transform infrastructure" to the cap. (not here, but post unification).

When returning Var for equivalent cases, we don't just get a foo=foo added before the scheme, but rather:

foo_local = foo
...
call sub_foo(foo=foo_local)
...
foo=foo_local

# Equivalent units #
##################
def m2_s_minus_2__to__J_kg_minus_1():
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just checking, does a unit string of m2 s-1 in the metadata files correspond to m2_s_minus_2 or m_2_s_minus_2 or m_plus_2_s_minus_2?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In think you mean m2 s-2 as the metadata string?
But yeah, until these units, we didn't have the case where there is an exponent is in the numerator for a supported conversion.
I feel adding _plus_ is overkill, and adding _ between the base and the exponent may be confusing. But I'm down for whatever is agreed upon.

Copy link
Collaborator

Choose a reason for hiding this comment

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

There should be an underscore between the unit and the exponent, but I agree that the additional _plus is not needed.

Copy link
Collaborator Author

@dustinswales dustinswales Jan 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@climbfuji After some sleuthing, I don't know if we should add the underscore here. We would need to extend units_to_string() to find and partition the baseXpositive_exponent piece of the string.
This is not hard, but we don't distinguish the baseXpositive_exponent anywhere else in the metadata (there are "m2" all over the physics, for example), so adding that distinction for the name of an internal procedures name is probably not necessary.

"""Equivalent units"""
return None

def J_kg_minus_1__to__m2_s_minus_2():
"""Equivalent units"""
return None
2 changes: 1 addition & 1 deletion scripts/var_props.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1426,7 +1426,7 @@ def has_unit_transforms(self):
and <var> arguments to produce code to transform one variable into
the correct units of the other.
"""
return self.__unit_transforms is not None
return self.__unit_transforms is not None and self.__unit_transforms[0]

def __bool__(self):
"""Return True if this object describes two Var objects which are
Expand Down
4 changes: 3 additions & 1 deletion test/var_compatibility_test/effr_calc.F90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ module effr_calc
!!
subroutine effr_calc_run(ncol, nlev, effrr_in, effrg_in, ncg_in, nci_out, &
effrl_inout, effri_out, effrs_inout, ncl_out, &
has_graupel, scalar_var, errmsg, errflg)
has_graupel, scalar_var, tke_inout, errmsg, errflg)

integer, intent(in) :: ncol
integer, intent(in) :: nlev
Expand All @@ -33,6 +33,8 @@ subroutine effr_calc_run(ncol, nlev, effrr_in, effrg_in, ncg_in, nci_out, &
character(len=512), intent(out) :: errmsg
integer, intent(out) :: errflg
real(kind_phys), intent(out),optional :: ncl_out(:,:)
real(kind_phys), intent(inout) :: tke_inout

!----------------------------------------------------------------

real(kind_phys), parameter :: re_qc_min = 2.5 ! microns
Expand Down
8 changes: 8 additions & 0 deletions test/var_compatibility_test/effr_calc.meta
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,14 @@
type = real
kind = kind_phys
intent = inout
[ tke_inout ]
standard_name = turbulent_kinetic_energy
long_name = turbulent_kinetic_energy
units = m2 s-2
dimensions = ()
type = real
kind = kind_phys
intent = inout
[ errmsg ]
standard_name = ccpp_error_message
long_name = Error message for error handling in CCPP
Expand Down
3 changes: 3 additions & 0 deletions test/var_compatibility_test/run_test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ required_vars_var_compatibility="${required_vars_var_compatibility},horizontal_d
required_vars_var_compatibility="${required_vars_var_compatibility},horizontal_loop_begin"
required_vars_var_compatibility="${required_vars_var_compatibility},horizontal_loop_end"
required_vars_var_compatibility="${required_vars_var_compatibility},scalar_variable_for_testing"
required_vars_var_compatibility="${required_vars_var_compatibility},turbulent_kinetic_energy"
required_vars_var_compatibility="${required_vars_var_compatibility},vertical_layer_dimension"
input_vars_var_compatibility="cloud_graupel_number_concentration"
#input_vars_var_compatibility="${input_vars_var_compatibility},cloud_ice_number_concentration"
Expand All @@ -157,13 +158,15 @@ input_vars_var_compatibility="${input_vars_var_compatibility},horizontal_dimensi
input_vars_var_compatibility="${input_vars_var_compatibility},horizontal_loop_begin"
input_vars_var_compatibility="${input_vars_var_compatibility},horizontal_loop_end"
input_vars_var_compatibility="${input_vars_var_compatibility},scalar_variable_for_testing"
input_vars_var_compatibility="${input_vars_var_compatibility},turbulent_kinetic_energy"
input_vars_var_compatibility="${input_vars_var_compatibility},vertical_layer_dimension"
output_vars_var_compatibility="ccpp_error_code,ccpp_error_message"
output_vars_var_compatibility="${output_vars_var_compatibility},cloud_ice_number_concentration"
output_vars_var_compatibility="${output_vars_var_compatibility},effective_radius_of_stratiform_cloud_ice_particle"
output_vars_var_compatibility="${output_vars_var_compatibility},effective_radius_of_stratiform_cloud_liquid_water_particle"
output_vars_var_compatibility="${output_vars_var_compatibility},effective_radius_of_stratiform_cloud_snow_particle"
output_vars_var_compatibility="${output_vars_var_compatibility},scalar_variable_for_testing"
output_vars_var_compatibility="${output_vars_var_compatibility},turbulent_kinetic_energy"


##
Expand Down
11 changes: 7 additions & 4 deletions test/var_compatibility_test/test_host.F90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -351,26 +351,28 @@ program test

character(len=cs), target :: test_parts1(1) = (/ 'radiation ' /)

character(len=cm), target :: test_invars1(8) = (/ &
character(len=cm), target :: test_invars1(9) = (/ &
'effective_radius_of_stratiform_cloud_rain_particle ', &
'effective_radius_of_stratiform_cloud_liquid_water_particle', &
'effective_radius_of_stratiform_cloud_snow_particle ', &
'effective_radius_of_stratiform_cloud_graupel ', &
'cloud_graupel_number_concentration ', &
'scalar_variable_for_testing ', &
'turbulent_kinetic_energy ', &
'flag_indicating_cloud_microphysics_has_graupel ', &
'flag_indicating_cloud_microphysics_has_ice '/)

character(len=cm), target :: test_outvars1(7) = (/ &
character(len=cm), target :: test_outvars1(8) = (/ &
'ccpp_error_code ', &
'ccpp_error_message ', &
'effective_radius_of_stratiform_cloud_ice_particle ', &
'effective_radius_of_stratiform_cloud_liquid_water_particle', &
'effective_radius_of_stratiform_cloud_snow_particle ', &
'cloud_ice_number_concentration ', &
'scalar_variable_for_testing ' /)
'scalar_variable_for_testing ', &
'turbulent_kinetic_energy '/)

character(len=cm), target :: test_reqvars1(12) = (/ &
character(len=cm), target :: test_reqvars1(13) = (/ &
'ccpp_error_code ', &
'ccpp_error_message ', &
'effective_radius_of_stratiform_cloud_rain_particle ', &
Expand All @@ -381,6 +383,7 @@ program test
'cloud_graupel_number_concentration ', &
'cloud_ice_number_concentration ', &
'scalar_variable_for_testing ', &
'turbulent_kinetic_energy ', &
'flag_indicating_cloud_microphysics_has_graupel ', &
'flag_indicating_cloud_microphysics_has_ice '/)

Expand Down
2 changes: 1 addition & 1 deletion test/var_compatibility_test/test_host_data.F90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ module test_host_data
effrg, & ! effective radius of cloud graupel
ncg, & ! number concentration of cloud graupel
nci ! number concentration of cloud ice
real(kind_phys) :: scalar_var
real(kind_phys) :: scalar_var, tke
end type physics_state

public allocate_physics_state
Expand Down
7 changes: 7 additions & 0 deletions test/var_compatibility_test/test_host_data.meta
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,3 +59,10 @@
dimensions = ()
type = real
kind = kind_phys
[ tke ]
standard_name = turbulent_kinetic_energy
long_name = turbulent_kinetic_energy
units = J kg-1
dimensions = ()
type = real
kind = kind_phys
1 change: 1 addition & 0 deletions test/var_compatibility_test/test_host_mod.F90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ subroutine init_data()
phys_state%effri = 5.0E-5 ! 50 microns, in meter
phys_state%nci = 80
endif
phys_state%tke = 10.0 !J kg-1

end subroutine init_data

Expand Down
4 changes: 3 additions & 1 deletion test/var_compatibility_test/test_reports.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,14 +74,16 @@ def usage(errmsg=None):
"effective_radius_of_stratiform_cloud_graupel",
"cloud_graupel_number_concentration",
"scalar_variable_for_testing",
"turbulent_kinetic_energy",
"flag_indicating_cloud_microphysics_has_graupel",
"flag_indicating_cloud_microphysics_has_ice"]
_OUTPUT_VARS_VAR_ACTION = ["ccpp_error_code", "ccpp_error_message",
"effective_radius_of_stratiform_cloud_ice_particle",
"effective_radius_of_stratiform_cloud_liquid_water_particle",
"effective_radius_of_stratiform_cloud_snow_particle",
"cloud_ice_number_concentration",
"scalar_variable_for_testing"]
"scalar_variable_for_testing",
"turbulent_kinetic_energy"]
_REQUIRED_VARS_VAR_ACTION = _INPUT_VARS_VAR_ACTION + _OUTPUT_VARS_VAR_ACTION

def fields_string(field_type, field_list, sep):
Expand Down
Loading