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

Capgen in SCM: Multiple instances of local_name in Group Cap #631

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

Capgen in SCM: Multiple instances of local_name in Group Cap #631

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f042d74
Bug fix and exercise change in testing.
dustinswales Jan 29, 2025
f2df258
Omission from previous commit
dustinswales Jan 29, 2025
67bfa95
Merge branch 'bugfix/scheme_ordering_in_caps' of https://github.com/d…
dustinswales Jan 29, 2025
7a24b29
Merge branch 'develop' into bugfix/local_var_names_in_caps
climbfuji Feb 3, 2025
dd2f30e
Change one test scalar to type int.
dustinswales Feb 4, 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
25 changes: 25 additions & 0 deletions scripts/metavar.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1677,12 +1677,37 @@ def add_variable(self, newvar, run_env, exists_ok=False, gen_unique=False,
context=newvar.context)
# end if
# end if
# Check if local_name exists in Group. If applicable, Create new
# variable with uniquie name. There are two instances when new names are
Comment on lines +1680 to +1681
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
# Check if local_name exists in Group. If applicable, Create new
# variable with uniquie name. There are two instances when new names are
# Check if local_name exists in Group. If applicable, create new
# variable with unique name. There are two instances when new names are

# created:
# - Same <local_name> used in different DDTs.
# - Different <standard_name> using the same <local_name> in a Group.
# During the Group analyze phase, <gen_unique> is True.
lname = newvar.get_prop_value('local_name')
lvar = self.find_local_name(lname)
if lvar is not None:
# Check if <lvar> is part of a different DDT than <newvar>.
# The API uses the full variable references when calling the Group Caps,
# <lvar.call_string(self))> and <newvar.call_string(self)>.
# Within the context of a full reference, it is allowable for local_names
# to be the same in different data containers.
newvar_callstr = newvar.call_string(self)
lvar_callstr = lvar.call_string(self)
if newvar_callstr and lvar_callstr:
if newvar_callstr != lvar_callstr:
if not gen_unique:
exists_ok = True
# end if
# end if
# end if
if gen_unique:
new_lname = self.new_internal_variable_name(prefix=lname)
newvar = newvar.clone(new_lname)
# Local_name needs to be the local_name for the new
# internal variable, otherwise multiple instances of the same
# local_name in the Group cap will all be overwritten with the
# same local_name
lname = new_lname
elif not exists_ok:
errstr = 'Invalid local_name: {} already registered{}'
cstr = context_string(lvar.source.context, with_comma=True)
Expand Down
3 changes: 2 additions & 1 deletion test/var_compatibility_test/effr_diag.F90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,10 @@ end subroutine effr_diag_init
!> \section arg_table_effr_diag_run Argument Table
!! \htmlinclude arg_table_effr_diag_run.html
!!
subroutine effr_diag_run( effrr_in, errmsg, errflg)
subroutine effr_diag_run( effrr_in, scalar_var, errmsg, errflg)

real(kind_phys), intent(in) :: effrr_in(:,:)
integer, intent(in) :: scalar_var
character(len=512), intent(out) :: errmsg
integer, intent(out) :: errflg
!----------------------------------------------------------------
Expand Down
7 changes: 7 additions & 0 deletions test/var_compatibility_test/effr_diag.meta
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,13 @@
kind = kind_phys
intent = in
top_at_one = True
[ scalar_var ]
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 probably my capgen ignorance showing, but I'm not seeing how these new test variables are checking the new logic. They aren't in DDTs and they aren't duplicate names with different standard names, unless I'm missing something?.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@mkavulich it's the variable naming in the caps. The test now uses same local_name for 3 different standard_names. To see this, run the var_compatabilty_test and look at the suite cap in ct_build/ccpp, you will see that each instance of scalar_var in the Cap has been renamed to a unique name, scalar_var1, Scalar_var2, and Scalar_var3, respectively.

standard_name = scalar_variable_for_testing_c
long_name = unused scalar variable C
units = m
dimensions = ()
type = integer
intent = in
[ errmsg ]
standard_name = ccpp_error_message
long_name = Error message for error handling in CCPP
Expand Down
3 changes: 2 additions & 1 deletion test/var_compatibility_test/effr_post.F90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,10 @@ end subroutine effr_post_init
!> \section arg_table_effr_post_run Argument Table
!! \htmlinclude arg_table_effr_post_run.html
!!
subroutine effr_post_run( effrr_inout, errmsg, errflg)
subroutine effr_post_run( effrr_inout, scalar_var, errmsg, errflg)

real(kind_phys), intent(inout) :: effrr_inout(:,:)
real(kind_phys), intent(in) :: scalar_var
character(len=512), intent(out) :: errmsg
integer, intent(out) :: errflg
!----------------------------------------------------------------
Expand Down
8 changes: 8 additions & 0 deletions test/var_compatibility_test/effr_post.meta
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,14 @@
type = real
kind = kind_phys
intent = inout
[ scalar_var ]
standard_name = scalar_variable_for_testing_b
long_name = unused scalar variable B
units = m
dimensions = ()
type = real
kind = kind_phys
intent = in
[ errmsg ]
standard_name = ccpp_error_message
long_name = Error message for error handling in CCPP
Expand Down
3 changes: 2 additions & 1 deletion test/var_compatibility_test/effr_pre.F90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,10 @@ end subroutine effr_pre_init
!> \section arg_table_effr_pre_run Argument Table
!! \htmlinclude arg_table_effr_pre_run.html
!!
subroutine effr_pre_run( effrr_inout, errmsg, errflg)
subroutine effr_pre_run( effrr_inout, scalar_var, errmsg, errflg)

real(kind_phys), intent(inout) :: effrr_inout(:,:)
real(kind_phys), intent(in) :: scalar_var
character(len=512), intent(out) :: errmsg
integer, intent(out) :: errflg
!----------------------------------------------------------------
Expand Down
8 changes: 8 additions & 0 deletions test/var_compatibility_test/effr_pre.meta
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,14 @@
type = real
kind = kind_phys
intent = inout
[ scalar_var ]
standard_name = scalar_variable_for_testing_a
long_name = unused scalar variable A
units = m
dimensions = ()
type = real
kind = kind_phys
intent = in
[ errmsg ]
standard_name = ccpp_error_message
long_name = Error message for error handling in CCPP
Expand Down
6 changes: 6 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,9 @@ 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},scalar_variable_for_testing_a"
required_vars_var_compatibility="${required_vars_var_compatibility},scalar_variable_for_testing_b"
required_vars_var_compatibility="${required_vars_var_compatibility},scalar_variable_for_testing_c"
required_vars_var_compatibility="${required_vars_var_compatibility},scheme_order_in_suite"
required_vars_var_compatibility="${required_vars_var_compatibility},vertical_layer_dimension"
input_vars_var_compatibility="cloud_graupel_number_concentration"
Expand All @@ -158,6 +161,9 @@ 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},scalar_variable_for_testing_a"
input_vars_var_compatibility="${input_vars_var_compatibility},scalar_variable_for_testing_b"
input_vars_var_compatibility="${input_vars_var_compatibility},scalar_variable_for_testing_c"
input_vars_var_compatibility="${input_vars_var_compatibility},scheme_order_in_suite"
input_vars_var_compatibility="${input_vars_var_compatibility},vertical_layer_dimension"
output_vars_var_compatibility="ccpp_error_code,ccpp_error_message"
Expand Down
10 changes: 8 additions & 2 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,13 +351,16 @@ program test

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

character(len=cm), target :: test_invars1(9) = (/ &
character(len=cm), target :: test_invars1(12) = (/ &
'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 ', &
'scalar_variable_for_testing_a ', &
'scalar_variable_for_testing_b ', &
'scalar_variable_for_testing_c ', &
'scheme_order_in_suite ', &
'flag_indicating_cloud_microphysics_has_graupel ', &
'flag_indicating_cloud_microphysics_has_ice '/)
Expand All @@ -374,7 +377,7 @@ program test
'scheme_order_in_suite '/)


character(len=cm), target :: test_reqvars1(13) = (/ &
character(len=cm), target :: test_reqvars1(16) = (/ &
'ccpp_error_code ', &
'ccpp_error_message ', &
'effective_radius_of_stratiform_cloud_rain_particle ', &
Expand All @@ -385,6 +388,9 @@ program test
'cloud_graupel_number_concentration ', &
'cloud_ice_number_concentration ', &
'scalar_variable_for_testing ', &
'scalar_variable_for_testing_a ', &
'scalar_variable_for_testing_b ', &
'scalar_variable_for_testing_c ', &
'scheme_order_in_suite ', &
'flag_indicating_cloud_microphysics_has_graupel ', &
'flag_indicating_cloud_microphysics_has_ice '/)
Expand Down
4 changes: 4 additions & 0 deletions 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 @@ -13,7 +13,11 @@ module test_host_data
ncg, & ! number concentration of cloud graupel
nci ! number concentration of cloud ice
real(kind_phys) :: scalar_var
real(kind_phys) :: scalar_varA
real(kind_phys) :: scalar_varB
integer :: scalar_varC
integer :: scheme_order

end type physics_state

public allocate_physics_state
Expand Down
20 changes: 20 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,6 +59,26 @@
dimensions = ()
type = real
kind = kind_phys
[scalar_varA]
standard_name = scalar_variable_for_testing_a
long_name = unused scalar variable A
units = m
dimensions = ()
type = real
kind = kind_phys
[scalar_varB]
standard_name = scalar_variable_for_testing_b
long_name = unused scalar variable B
units = m
dimensions = ()
type = real
kind = kind_phys
[scalar_varC]
standard_name = scalar_variable_for_testing_c
long_name = unused scalar variable C
units = m
dimensions = ()
type = integer
[scheme_order]
standard_name = scheme_order_in_suite
long_name = scheme order in suite definition file
Expand Down
3 changes: 3 additions & 0 deletions 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,6 +74,9 @@ def usage(errmsg=None):
"effective_radius_of_stratiform_cloud_graupel",
"cloud_graupel_number_concentration",
"scalar_variable_for_testing",
"scalar_variable_for_testing_a",
"scalar_variable_for_testing_b",
"scalar_variable_for_testing_c",
"scheme_order_in_suite",
"flag_indicating_cloud_microphysics_has_graupel",
"flag_indicating_cloud_microphysics_has_ice"]
Expand Down