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

Add dependencies to metadata #317

Merged
merged 8 commits into from
Sep 2, 2020
Merged

Conversation

climbfuji
Copy link
Collaborator

@climbfuji climbfuji commented Aug 4, 2020

This PR fixes #308 for ccpp_prebuild.py.

Changes:

  • add capability to parse new, required sections starting with [ccpp-table-properties]
    • the names in the name = ... attributes for these sections cannot end with one of the predefined CCPP stages (e.g. _init)
    • the type for these sections must be the same table as the corresponding [ccpp-arg-table] sections
  • remove unnecessary duplication of information in ccpp_prebuild.py's argument dictionary: instead of arguments[module_name][scheme_name][subroutine_name], use arguments[scheme_name][subroutine_name]; this is sufficient, because per requirement module_name = scheme_name
  • fix a bug in src/ccpp_types.meta
  • fix a warning in scripts/metavar.py by changing is to ==:
FV3/ccpp/framework/scripts/metavar.py:423: SyntaxWarning: "is" with a literal. Did you mean "=="?
  if source.type is 'SCHEME':

Associated PRs:
earth-system-radiation/rte-rrtmgp#85
#317
NCAR/ccpp-physics#483
NOAA-EMC/fv3atm#153
ufs-community/ufs-weather-model#180

See ufs-community/ufs-weather-model#180 for regression testing.

@climbfuji
Copy link
Collaborator Author

@gold2718 @grantfirl @ligiabernardet @llpcarson please have a look at this attempt to implement the CCPP scheme dependencies in the metadata.

@gold2718 since your version of the metadata parser metadata_table.py has advanced, and since the old version used in the master branch works in conjunction with ccpp_prebuild.py's metadata_parser.py, we are more or less free how to implement this under the hood. As long as the input (see suggested format in the ccpp-physics PR NCAR/ccpp-physics#483) is the same for ccpp_prebuild.py and cap_gen.py, we are good.

…data header is a property table or not to scripts/metadata_table.py
@climbfuji climbfuji marked this pull request as ready for review August 12, 2020 20:38
@gold2718
Copy link
Collaborator

add capability to parse sections starting with [ccpp-table-properties]

It seems that this section is required, can that be added to the statement above? (e.g., 'parse required sections')

@climbfuji
Copy link
Collaborator Author

add capability to parse sections starting with [ccpp-table-properties]

It seems that this section is required, can that be added to the statement above? (e.g., 'parse required sections')

Done.

@climbfuji
Copy link
Collaborator Author

@grantfirl @llpcarson @gold2718 please review this PR, if possible. @ligiabernardet @JulieSchramm please also take a look for your information and future documentation updates

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.

This looks OK to me, relying on the tests to confirm (vs code inspection!)

Copy link
Collaborator

@grantfirl grantfirl left a comment

Choose a reason for hiding this comment

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

I didn't find anything obviously amiss. Nice job on the implementation!

@climbfuji
Copy link
Collaborator Author

@gold2718 I will need to merge this now in order to not hold back the UFS, sorry.

@climbfuji climbfuji merged commit 8365587 into NCAR:master Sep 2, 2020
@gold2718
Copy link
Collaborator

gold2718 commented Sep 2, 2020

@climbfuji, Sorry for not getting through this. To be honest, I do not understand the code and have been struggling to get through it.
I'll take another look but as long as we agree on the semantics, it should be okay in the long run.

@climbfuji
Copy link
Collaborator Author

@climbfuji, Sorry for not getting through this. To be honest, I do not understand the code and have been struggling to get through it.
I'll take another look but as long as we agree on the semantics, it should be okay in the long run.

Thansk @gold2718 - that was my understanding, too. As long as the input (semantics) and the output (functional code) are correct, it shouldn't matter. Once we switch to cap_gen.py, you will understand the code, hopefully I will, too.

DusanJovic-NOAA pushed a commit to NOAA-EMC/fv3atm that referenced this pull request Sep 2, 2020
This PR removes dependency information from the CCPP prebuild config. See NCAR/ccpp-framework#308 and NCAR/ccpp-framework#317 for details on the motivation for this change and the actual implementation. It also removes some legacy code used by the dynamic CCPP build in the past.

This PR also contains the changes in #156, i.e. the completion of adding the active attribute to GFS_typedefs.F90. On top of this PR, the missing active attribute for phy_fctd is added.

Additionally, gfortran-10 compiler flags are added to CCPP's CMakeLists.txt.
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.

Metadata should contain dependency information for CCPP scheme
4 participants