NCAR / ccpp-framework

Common Community Physics Package (CCPP)
http://www.dtcenter.org/community-code/common-community-physics-package-ccpp/
Other
26 stars 64 forks source link

Capgen to adopt/improve on DDT metadata from prebuild #377

Open climbfuji opened 3 years ago

climbfuji commented 3 years ago

Description

ccpp_prebuild.py defines DDTs and instances of it as shown here:

Typedefs: https://github.com/NOAA-EMC/fv3atm/blob/develop/ccpp/data/CCPP_typedefs.meta https://github.com/NOAA-EMC/fv3atm/blob/develop/ccpp/data/CCPP_typedefs.F90

Instance: https://github.com/NOAA-EMC/fv3atm/blob/develop/ccpp/data/CCPP_data.meta https://github.com/NOAA-EMC/fv3atm/blob/develop/ccpp/data/CCPP_data.F90

Solution

capgen should use the same or a similar/better approach.

gold2718 commented 3 years ago

I am confused on some of the usage. In CCPP_data.meta, I see:

[CCPP_interstitial]
  standard_name = CCPP_interstitial_type_instance
  long_name = instance of derived type CCPP_interstitial_type
  units = DDT
  dimensions = ()
  type = CCPP_interstitial_type

which I would interpret as a scalar DDT (no dimensions). That seems to be true in the code.

However, later in the file, I see:

[GFS_Interstitial(ccpp_thread_number)]
  standard_name = GFS_interstitial_type_instance
  long_name = instance of derived type GFS_interstitial_type
  units = DDT
  dimensions = ()
  type = GFS_interstitial_type

which I intrepret as an element of an array of type, GFS_interstitial_type, however, the code just lists the variable as an array. Would you be okay if we omitted that and just had the metadata for GFS_Interstitial (last in the file) be:

[GFS_Interstitial]
  standard_name = GFS_interstitial_type_instance
  long_name = instance of derived type GFS_interstitial_type
  units = DDT
  dimensions = (omp_threads)
  type = GFS_interstitial_type
climbfuji commented 3 years ago

I am confused on some of the usage. In CCPP_data.meta, I see:

[CCPP_interstitial]
  standard_name = CCPP_interstitial_type_instance
  long_name = instance of derived type CCPP_interstitial_type
  units = DDT
  dimensions = ()
  type = CCPP_interstitial_type

which I would interpret as a scalar DDT (no dimensions). That seems to be true in the code.

However, later in the file, I see:

[GFS_Interstitial(ccpp_thread_number)]
  standard_name = GFS_interstitial_type_instance
  long_name = instance of derived type GFS_interstitial_type
  units = DDT
  dimensions = ()
  type = GFS_interstitial_type

which I intrepret as an element of an array of type, GFS_interstitial_type, however, the code just lists the variable as an array. Would you be okay if we omitted that and just had the metadata for GFS_Interstitial (last in the file) be:

[GFS_Interstitial]
  standard_name = GFS_interstitial_type_instance
  long_name = instance of derived type GFS_interstitial_type
  units = DDT
  dimensions = (omp_threads)
  type = GFS_interstitial_type

These are two different DDTs - one is GFS_interstitial, one is CCPP_interstitial.

GFS_interstitial is an array of length (omp_threads). In order to call a scheme, one needs to pass the correct element = scalar to the MPI task/OpenMP thread. The scalar is therefore defined as GFS_Interstitial(ccpp_thread_number). The entire array is GFS_Interstitial and, in this setting, would have a standard name GFS_interstitial_type_instance_all_threads.

CCPP_interstitial is a scalar, since there is no threading (and no blocking).

gold2718 commented 3 years ago

I do not see a problem with CCPP_interstitial, it is an example of what I expect to see. My problem with GFS_Interstitial is I do not understand why there are two entries for one chunk of memory and why are there two different standard names for the same 'physical quantity' (which seems to violate the CCPP rule of 1-1 correspondence between standard name and physical quantity). Is the need for this fundamental in the UFS or just a quirk of the prebuild implementation?

climbfuji commented 3 years ago

I don't know if it is a quirk. But when you call the physics, you need pass certain variables that belong to a specific OpenMP thread to the physics. How does capgen know which element of the GFS_Interstitial array to pass to the call for OpenMP thread 0, 1, .. ? If it has a way to do this, then we don't need the extra logic anymore.

gold2718 commented 3 years ago

I would advocate using a host-model variable for the 'chunk' of data to pass in a particular call to ccpp_physics_run. If you really want to use OMP and are calling from within a threaded region, you can assign the variable to omp_get_thread_num(). The point is that if you have a variable for the chunk index (and I would prefer a general approach that does not use a standard name specific to OMP), then capgen can generate the correct code. This is how it selects a portion of an array using horizontal_loop_begin and horizontal_loop_end.

climbfuji commented 3 years ago

I guess I need to see this in action to fully understand.

mkavulich commented 2 years ago

Is this resolved by #383?