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.

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

Static build: no cdata to store and retrieve variables #168

Merged
merged 7 commits into from
Jan 28, 2019

Conversation

climbfuji
Copy link
Collaborator

@climbfuji climbfuji commented Jan 23, 2019

This PR and associated PRs remove the need to store host model variables in the cdata structure and retrieve them in the physics for the static build only.

For this to work, and in the absence of a central host model registry in FV3, the following major changes are required:

  • the auto-generated ccpp_static_api.F90 is no longer written to ccpp/physics/physics and compiled into the static CCPP physics library, but instead to FV3/gfsphysics/CCPP_layer and compiled with the host model
  • the host model variables are imported in the ccpp_static_api module instead of being passed in via the argument list - this allows to maintain the same API for the static and dynamic builds
  • in order to retrieve the correct block and thread number inside physics, these are now attributes of cdata (of type ccpp_t), similar to the loop counter, error message and error flag
  • by passing the correct cdata (either a scalar or a single member of an array) to the CCPP API (static or dynamic), the system has all required information for all build types
  • store the GFS data types required in FV3/gfsphysics/CCPP_data.F90, import them as IPD data types in atmos_model.F90
  • do not use variable Atm (of fv_atmos_type) directly for the CCPP fast physics, but instead add the required variables to CCPP_interstitial (in dynamics) - for the sake of performance and memory footprint, use pointers from CCPP_interstitial to Atm; using fv_atmos_type in CCPP seems to be the reason for the GNU compiler crashes in the current gmtb/develop version

Further, these PRs remove several of the hard-coded module use statements, previously defined in ccpp_prebuild_config.py, which required

  • defining how kind types and derived data types are declared in the metadata: kind definitions require local_name, standard_name and kind attributes to match; similarly, type definitions require local_name, standard_name and type attributes to match
  • cleaning up the inconsistent definition of the FV3 GFS DDTs in the metadata tables so that type definitions and definitions of instances of a DDT are kept separate

These PRs are also one step towards removing the IPD legacy. Once the hybrid build is removed, the remaining call to IPD_initialize can be substituted by a direct call of GFS_initialize (or some other method, to be discussed) and IPD_Control etc can be renamed into GFS_Control etc.

…variable as a kind or type definition (as opposed to an instance), add new print statement using intents instead of pointers for static build; modification of Cap class: auto-generate module use information instead of receiving it from hard-coded value in CCPP prebuild config
…eceive all required arguments (parents/DDTs of variables that are members of DDTs, individual variables, indices) via the argument list instead of pulling them out of cdata; modify API to import the required variables from the host model instead of getting them from cdata (this requires to move the ccpp_static_api.F90 compilation into the host model)
…s to subroutine calls for static build w/o using cdata, and for dynamic build without using hard-coded module use statements
@climbfuji climbfuji changed the base branch from master to gmtb/develop January 23, 2019 17:05
…onfig to work with new static build version of ccpp_prebuild.py
@climbfuji
Copy link
Collaborator Author

@climbfuji
Copy link
Collaborator Author

Note: these changes also fix the GNU compiler crash when compiling NEMSfv3gfs with CCPP (introduced by https://github.com/NCAR/NEMSfv3gfs/pull/88 and associated PRs).

@climbfuji
Copy link
Collaborator Author

Testing: these changes were tested for bit-for-bit identical results for all build options manually on MacOSX/GNU for fv3_control and fv3_gfdlmp, and on Cheyenne/Intel for fv3_gfdlmp.

@climbfuji
Copy link
Collaborator Author

For a better understanding, here are auto-generated caps for the various groups and the suite (in ccpp/physics/physics), for the GFDLMP suite (I had to add .txt to be able to attach them):

ccpp_group_fast_physics_cap.F90.txt
ccpp_group_physics_cap.F90.txt
ccpp_group_radiation_cap.F90.txt
ccpp_group_stochastics_cap.F90.txt
ccpp_group_time_vary_cap.F90.txt
ccpp_suite_cap.F90.txt

This is the auto-generated static API (in FV3/gfsphysics/CCPP_layer) for the same suite (and with .txt at the end):

ccpp_static_api.F90.txt

@climbfuji climbfuji closed this Jan 25, 2019
@climbfuji climbfuji changed the base branch from gmtb/develop to master January 25, 2019 21:37
@climbfuji
Copy link
Collaborator Author

Reopen, change target to master branch

@climbfuji climbfuji reopened this Jan 25, 2019
@codecov-io
Copy link

codecov-io commented Jan 25, 2019

Codecov Report

Merging #168 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #168   +/-   ##
=======================================
  Coverage   44.96%   44.96%           
=======================================
  Files          16       16           
  Lines        1401     1401           
=======================================
  Hits          630      630           
  Misses        771      771
Impacted Files Coverage Δ
src/ccpp_types.F90 75% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b88766e...e94b3b9. Read the comment docs.

@climbfuji
Copy link
Collaborator Author

Update: in order to deal with some of the difficulties around the missing data-laundering functionality in the static/no-cdata version, and to accommodate the bit-for-bit modifications in the next weeks, updates have been pushed to the different PRs that make use of a new NEMSfv3gfs MAKEOPT (TRANSITION=Y, default is N), which also translates into a CPP directive -DTRANSITION. This new MAKEOPT and the CPP directive will allow making selective modifications of the code to achieve bit-for-bit identical results on Theia with Intel 15 in PROD mode.

These updates require rerunning all relevant tests, I am thus deleting the above regression test logs.

@climbfuji
Copy link
Collaborator Author

Output of standard regression test logs (Theia, Intel 18, REPRO mode). All tests pass.

rt_ccpp_hybrid.log
rt_ccpp_ref_create.log
rt_ccpp_standalone.log
rt_ccpp_static.log
rt_full.log

@climbfuji
Copy link
Collaborator Author

Output of CCPP acceptance tests (Theia, Intel 15, PROD mode). The result is the same as before introducing the new static build w/o cdata: The basic fv3_control tests pass, all other tests fail (in particular the GFDL-MP based tests). This means that this PR and related PRs are good to merge.

rt_ccpp_ref_for_acceptance_create.log
rt_ccpp_static_for_acceptance.log

Copy link
Contributor

@llpcarson llpcarson left a comment

Choose a reason for hiding this comment

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

Approved

Copy link
Collaborator

@gold2718 gold2718 left a comment

Choose a reason for hiding this comment

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

I am still not super clear on what specific problem this PR solves or where this is headed but nothing pops out at me as a problem.

@climbfuji
Copy link
Collaborator Author

Thanks Laurie and Steve for approving. The purpose of this PR was to remove the need for using cdata to store variables from the host model in the lookup table "standard_name" -> "location in memory". There are two reasons for making this change, (1) performance and (2) preparation for using capgen in the future. As we have seen at the last meeting, the auto-generated caps of this new no-cdata static build and of capgen are quite similar. Several of the issues addressed in this PR here would have come up in the future when switching over to capgen. Doing it here allows us to split this huge upcoming change into smaller chunks.

@climbfuji climbfuji merged commit ec403ba into NCAR:master Jan 28, 2019
@climbfuji climbfuji deleted the no-cdata-for-static-build branch June 27, 2022 02:53
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