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

CMake refinements #317

Open
7 of 12 tasks
RobertPincus opened this issue Dec 23, 2024 · 3 comments
Open
7 of 12 tasks

CMake refinements #317

RobertPincus opened this issue Dec 23, 2024 · 3 comments

Comments

@RobertPincus
Copy link
Member

RobertPincus commented Dec 23, 2024

Issues noted by @skosukhin outstanding from #316

  • Come up with a common prefix for custom CMake variables. For example, BUILD_TESTING is a standard variable and should keep the name but KERNEL_MODE, FAILURE_THRESHOLD, RTE_ENABLE_SP, etc. are project-specific and should have the same and a project-specific prefix. Usually, this would be the name of the project but RTE_RRTMGP_ seems to be rather long. Maybe we could use something like RR_.
  • The tests are currently missing dependencies on each other and, therefore, we cannot run them in parallel.
  • Introduce the missing test FIXTURES and consider fetching the required data at the build time as one of the tests.
  • Make it possible to install the libraries (and maybe the data files).
  • Come up with the public aliases for the targets (e.g. rte-rrtmgp:rte, rte-rrtmgp:rte-dp, etc.).
  • Make sure we can build rte-rrtmgp as a subproject (i.e. add_subdirectory(rte-rrtmgp) should work and expose the targets).
  • Make sure that a superproject can find rte-rrtmgp (i.e. find_project(rte-rrtmgp)) in the build directory, as well as in the installation prefix.
  • Consider changing the directory layout. For example, it feels natural to move rte-kernels and rte-frontend to a new first-level subdirectory rte. Directories gas-optics, rrtmgp-kernels and rrtmgp-frontend could be moved to a new first-level subdirectory rrtmgp.
  • We also have a minor inconsistency: the kernel mode is named extern but the respective files are stored in the api subdirectory.
  • Revisit .gitignore.
  • Extend formatting jobs to cover Python (isort, black, flake8), C/C++/CUDA (cmake-format) and Fortran (fprettify) source files.
  • Remove setup.sh (unconventional and misleading) and provide the building instructions in README.md (or elsewhere)
@RobertPincus
Copy link
Member Author

RobertPincus commented Dec 23, 2024

A note on the directory layout: gas_optics belongs to rte; rrtmgp is one implementation.

Also rte and rrtmgp may become separate repos in the not-too-distant future.

@RobertPincus
Copy link
Member Author

Two issues resolved in #319

@RobertPincus
Copy link
Member Author

Four issues resolved by #323

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

No branches or pull requests

1 participant