- Where an ExaGO policy exists, follow it.
- Otherwise, where a clear precedent exists in the project, follow it.
- Otherwise, where a good public C++ style guide is relevant and clear, follow it. Google's is very good and comes with justification for its rules. Flang's is also very thorough and is heavily referenced here.
- Policies specific to ExaGO are numbered so they may be referenced elsewhere. If someone or a tool refers to ExaGO developer policy P157, that refers to policy number 157 in this document.
To get started contributing to ExaGO, check out a feature or fix branch off of
the develop branch:
$ cd exago
$ git checkout develop
$ git checkout -b my-cool-feature-devTo submit a merge request, please make sure your branch is up to date with current development like so:
$ git pull --rebase origin developThis will ensure you are rebased on the most recent development branch. If you see open merge requests on ExaGO's repository that touch the same lines of code that you are working on, please coordinate with the developers of that merge request so your work doesn't conflict.
If your commit only touches comments and/or documentation, please add [skip ci] to your commit message.
This reduces unneeded burden on our CI system.
Use PascalCase for functions and classes, and snake_case for variables.
Prefer enum class over a bare enum.
Use constexpr, static, and const wherever appropriate. It is good practice to make all variables and functions static, const, and/or constexpr by default, and only remove these qualifiers when you have good reason to do so.
Use auto to declare a variable when the type is obvious. For example, when dynamic casting from a pointer, there is no need to specify the type twice:
auto *my_var = dynamic_cast<MyType*>(my_other_var);Functions should usually be less than 50 lines of code. If they are longer, consider breaking the function down into smaller functions.
Use anonymous namespaces for functions and variables which should have internal linkage.
It's common practice in C to mark these functions static, however it is preferred to use anonymous namespaces for this in ExaGO.
Don't use dynamic solutions to solve problems that can be solved at build time; don't solve build time problems by writing programs that produce source code when macros and templates suffice; don't write macros when templates suffice.
Never use more than 80 characters per source line, unless you're editing documentation.
Use clang-format (or the utility script buildsystem/tools/clang-format.pl with the -i flag) to format your code.
Example usage of the utility script:
$ cd exago/build
$ # do some development
$ make && ctest
$ # format ExaGO source in-place
$ ../buildsystem/tools/all.pl -i
$ # check to make sure everything is formatted (no flags means check if already formatted):
$ ../buildsystem/tools/all.plThis will format all the C++ source code in the ExaGO source tree.
There are some instances in which is makes sense to go over 80 characters per line, or to otherwise contradict our clang format configuration.
If this is the case, add // clang-format off before the snippet and // clang-format on after.
For the vast majority of cases, just use the clang-format configuration.
This widely referenced article on commit messages
goes into greater detail and describes other best practices that are also recommended.
The commit message should read: If applied, this commit will <commit message>.
Merge request titles should also follow the same conventions.
If your merge request changes only documentation, you may add [skip ci] to your commit message to skip CI.
Otherwise, your MR must pass our continuous integration pipelines before being merged.
CI tests rely on Spack generated tcl modules in order to build and test ExaGO on target platforms. In order to rebuild the modules for a given platform to update CI environments, you must do the following in an active MR (this workflow does not work in branches):
- Update the spack submodule in /tpl/spack to point to your desired spack commit. This may be necessary when incoporating new software versions, or when undertaking spack development.
- Update the relevant
spack.yaml(s)in /buildsystem/spack/ for each platform that you are building for. Currently only [Newell, Deception, Ascent] support automatic re-building. - Add a commit to to your MR with
[<clusterame>-rebuild]where<clustername>is replaced by and desired platform. Add multiple[<clusterame>-rebuild]into the commit message for each platform you want to rebuild for.
This should be done semi-regularly in order to incorporate new versions of packages, and to update ExaGO's package.py with any new conflicts found in spack's develop branch.
IMPORTANT: Patches should be rather small. Extremely large merge requests are helpful as proofs-of-concept, but when merging into key development branches, large merge requests should be broken into smaller ones. Smaller merge requests are easier to review and test, and when something goes wrong it's much easier to track down a bug. Rarely is a patch too small to constitute it's own MR. A notable exception to this rule is when merging key development branches back into develop and/or master. It's hard to avoid large MRs in this case.
Each MR should usually have a corresponding issue.
The release checklist is linked here. Refer to this document for release documentation.
This project adheres to the Keep-a-Changelog guidelines.
When making significant changes, summarize the changes in CHANGELOG.md under the develop or unreleased header.
Header files should be idempotent.
Header files should use header guards, particularly #pragma once over other methods of header guards.
Use underscores (_) and all lowercase characters for C, C++, and Python filenames and paths.
Use pascal case for CMake code.
Use the .h extension for all C and C++ headers.
Use the .cpp extension for all C++ sources.
Use the .def extension for all X-Macro headers.
All binaries, applications, drivers, tests, and libraries should use underscores (_) and all lowercase characters.
Use target-oriented cmake wherever possible. The following table provides a mapping from legacy CMake conventions to modern CMake:
- This article provides a helpful overview of target-oriented CMake.
- This article on modern CMake provides a basis for good CMake code.
When working with large lists of source files, create a list of source files before creating the target and then pass that list to add_library or add_executable.
The following are reasons this policy is helpful:
- Many times, when working with many source files for a single target, we will eventually run into a use case where we need to conditionally add a source file to the target. Using
target_sourcesto add source files when the target has already been created can be harder to read. Simply create a list of sources and conditionally calllist(APPEND MYSRCLIST mysource.cpp)before creating the target withadd_library(foo ${MYSRCLIST}). - When creating a large target, it is sometimes helpful to provide a secondary name for the collection of sources that do not make up an entire library of their own. For example, in
src/opflow/CMakeLists.txt, we create several lists of sources, the CMake listOPFLOW_SOLVER_SRCcontains the source files needed for the OPFLOW solver. The lists are then all concatenated at the bottom when the target is created. This makes our intent clearer. - When sources are already collected into lists, it is very simple to change the source language, which is needed for the portable GPU components of ExaGO. It is simple to set any property on a colletion of source files if they are already collected into a list. For example, in this CMake file in HiOp's codebase we can simply set all the GPU-portable sources to source language CUDA when CUDA and RAJA are enabled like so:
set_source_files_properties(${hiopLinAlg_RAJA_SRC} PROPERTIES LANGUAGE CUDA)The CMake wrapper function exago_add_library handles shared/static libraries consistently across all ExaGO libraries, and should be used over the usual add_library.
When linking libraries to a target, use PRIVATE whenever possible.
This prevents linking against libraries we don't have to link against, and makes our exported targets cleaner.
A .cmake-format.py file exists in the top-level directory - if you change any cmake code, please run cmake-format -i <file name> before pushing your changes.
You may also run buildsystem/tools/cmake_format.pl -i.