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 find-package modules for GPTL, PnetCDF, udunits (unify JCSDA and EMC cmakemodules) #64

Merged

Conversation

climbfuji
Copy link
Collaborator

@climbfuji climbfuji commented Jul 26, 2022

My goal is to unify the JCSDA and EMC CMakeModules, and for this I need to add a few find-package modules that exist in https://github.com/jcsda/jedi-cmake to this repository:

  • GPTL
  • PnetCDF (not sure if this is the right capitalization, but that's what it is called in jedi-cmake)
  • udunits (should this all be uppercase?)

Almost all of the existing files are identical between the two repos. There are tiny differences between the FindNetCDF.cmake files, but they are just additional comments and don't matter.

There are, however, large differences between the FindPIO.cmake files. My strategy is to test first if the version in this repository (CMakeModules) work just fine, and if so not care about the differences. If it doesn't, come back and revisit the FindPIO.cmake differences in a follow-up PR.

Question

Do the copyright statements at the top of the new files matter? I assume not, since the existing FindNetCDF.cmake file has an ECMWF copyright statement at the top.

Testing

This PR needs to be tested with jedi-cmake (which will be using this repo as a submodule). Requires installing a test version of jedi-cmake using spack-stack and then running our JEDI tests (with MPAS, FV3, ...).

Issues

First step (maybe the only step required) towards #65

@climbfuji climbfuji force-pushed the feature/add_find_gptl_pnetcdf_udunits branch from 555f200 to 406705b Compare July 26, 2022 19:05
@climbfuji climbfuji marked this pull request as ready for review July 26, 2022 19:13
Copy link
Collaborator

@aerorahul aerorahul left a comment

Choose a reason for hiding this comment

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

Looks good.

@climbfuji
Copy link
Collaborator Author

Update: I updated jedi-cmake (make CMakeModules a submodule and delete the existing findXYZ.make files) and tested it in several ways on my macOS:

  1. Build jedi-bundle with jedi-cmake being installed via spack-stack and loaded as module
  2. Build jedi-bundle with jedi-cmake NOT being installed via spack-stack and loaded as module, i.e. it gets built as part of the bundle
  3. Build mpas-bundle with jedi-cmake being installed via spack-stack and loaded as module (mainly for testing if the new findPIO.cmake works).

All of these worked fine, including finding PIO. I think we can merge this.

@climbfuji
Copy link
Collaborator Author

@kgerheiser Do you want to take a look?

Copy link
Contributor

@kgerheiser kgerheiser left a comment

Choose a reason for hiding this comment

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

Looks good

@climbfuji climbfuji merged commit cabd775 into NOAA-EMC:develop Jul 27, 2022
@climbfuji climbfuji deleted the feature/add_find_gptl_pnetcdf_udunits branch July 27, 2022 21:26
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.

3 participants