Making a Pull Request

Making a Pull Request

Checks to Run

Before making a PR please run clang-format, clang-tidy, and the tests to verify your code is properly formatted and without major errors. We use LLVM v17.0.1 at the time of writing.

Tests

Instructions for running the tests can be found in the Testing page.

Running Clang Tools Locally

There are essentially 2 discrete ways to run the clang-tools locally (i) manually installing and invoking the tools or (ii) invoking the pre-commit software. If you just care about running clang-format, the latter approach can be much more convenient.

Installation

System	Recommendation
MacOS	Homebrew
Linux	LLVM Packages page
Docker	Docker images
Summit	module load llvm/16.0.0
Crusher/Frontier	module load llvm/17.0.1 (If this package is unavailable then choose the latest LLVM v17 build)
Unix without root/sudo	python -m pip install clang-format==17.0.1 clang-tools --user

Generally the Linux install instructions simplify to

wget https://apt.llvm.org/llvm.sh
chmod +x llvm.sh
sudo ./llvm.sh <version number> all

Once installed make sure that the clang tools are on your path. You can typically do this by adding export PATH="/usr/lib/llvm-<version>/bin:$PATH" to your .bashrc or similar file for your shell. If LLVM was installed in a different location then you will need to add that path instead.

WARNING: Later versions of clang-format produce different output compared with earlier versions of clang-format, so you must exactly specify the version currently being used in our Actions. Both clang-format and clang-tidy can be integrated into most common IDEs/editors reasonably easily.

Currently Cholla has been tested with v17.0.1 of the Clang tools. Later versions should work but earlier versions may not.

Clang Format

clang-format can be run on the entire codebase either by running the tools/clang-format_runner.sh script or running make format. This can be added as a precommit hook by running echo "make format" >> [PATH_TO_CHOLLA]/.git/hooks/pre-commit.

Clang Tidy

clang-tidy can be run by running make tidy. Note that it can take 5-10 minutes to run and possibly longer on a single core machine. By default make tidy runs two instances of clang-tidy in parallel, one each for the C++ and CUDA files. It then writes the results to two files named tidy_results_cpp_(MAKE_TYPE).log and tidy_results_gpu_(MAKE_TYPE).log,

Extra arguments can be passed to clang tidy with the CLANG_TIDY_ARGS make variables. This can be used to run only a single check or subset of checks. Example: make tidy TYPE=hydro CLANG_TIDY_ARGS="--checks=-*,bugprone-assignment-in-if-condition --list-checks" runs clang tidy on all files with the hydro build, turns off all checks (the -*) then enables the bugprone-assignment-in-if-condition check and lists all checks to be run without actually running any of them (the --list-checks).

To only run clang-tidy on certain files you must use the TIDY_FILES make variable. Note that you must use the same path for the file that Make uses (e.g. src/main.cpp not main.cpp) and multiple files must be separated by a space with the whole list in quote marks. Examples:

• make tidy TIDY_FILES="src/main.cpp src/riemann_solvers/hlld_cuda.cu" will run clang-tidy on main.cpp and hlld_cuda.cu
• make tidy TIDY_FILES="$(git diff --name-only HEAD HEAD~N)" will run clang-tidy on all files that were changed in the last N commits
• make tidy TIDY_FILES="$(git diff --name-only HEAD $(git merge-base ROOT_BRANCH $(git branch --show-current)))" will run clang-tidy on all files changed since this branch branched off of ROOT_BRANCH

Invoking through pre-commit software

The pre-commit software is software written in python that is designed to provide a framework for attaching arbitrary sets of linting tools to git's commit-hooks. It is designed to easily download and manage isolated copies of third-party tools (like clang-format). To use pre-commit to invoke clang-format you need to install it (with pip) and then invoke the following from the root of the Grackle directory:

pre-commit run --all-files

The above command will install local copies of clang-format (they will be manage locally by pre-commit and the software is cached) and will then modify the files in your repository.

Style Guide

Cholla developers are asked to follow our style guide for naming and formatting in their contributions. Certain naming and all formatting conventions are automatically enforced by clang-tidy and clang-format, respectively. We also have standardized names for common physics variables. Naming for physics variables is not enforceable by clang-tidy, but please try to follow them as much as possible.

Naming

Enforced by clang-tidy

• Functions: Loud_Snake_Case
• Variables: snake_case
• Class/struct definitions: PascalCase
• Private variables: _ suffix, e.g. snake_case_
• Filenames: snake_case
• Host-resident variables: host prefix, e.g. host_snake_case
• Device-resident variables: dev prefix, e.g. dev_snake_case
• Global variables: g prefix, e.g. g_snake_case

Physics Variables

• Mass: density
• Momentum: momentum
• Velocity: velocity
• Vector components: suffix specifying direction, e.g. velocity_x
• Energy: energy, gas_energy, kinetic_energy magnetic_energy, total_energy
• Pressure: pressure, gas_pressure, magnetic_pressure
• Scalar: scalar
• Temperature: temperature
• Magnetic field: magnetic
• Electric field: electric
• Number density: number_density
• CUDA/index IDs: thread_id, id_x, id_y, id_z
• Log, sine, etc. of a variable: log, sine, etc. prefix, e.g. log_density
• Specific quantities: specific prefix, e.g. specific_energy
• Wave speeds: speed with suffix specifying name, e.g. speed_alfven
• Flux: field name with flux suffix, e.g. density_flux
  • Integrator flux arrays are simply flux with the appropriate directional suffix, e.g. flux_x
• Cell dimensions: dx, dy, dz
• Timestep: dt
• Inverse timestep: dti

Please include citations for any model equations, etc. used in your contribution.

Formatting

The full list of specifications can be found in the .clang-format file. Some examples of our formatting conventions are:

• Two spaces for indentation
• Lines should not exceed 120 characters
• Curly braces should be on their own line, e.g.

// i.e. this
void func(args)
{
  stuff
}

// not this
void func(args) {
  stuff
}

with an exception for conditional statements. Conditional statements should follow:

if (condition) {
  stuff
} else {
  other stuff
}

• Consecutive assignments should be aligned, e.g.

// This
int aaaa = 12;
int b    = 23;
int ccc  = 23;

#define SHORT_NAME       42
#define LONGER_NAME      0x007f
#define EVEN_LONGER_NAME (2)
#define foo(x)           (x * x)
#define bar(y, z)        (y + z)

// Not this
int aaaa = 12;
int b = 23;
int ccc = 23;

#define SHORT_NAME 42
#define LONGER_NAME 0x007f
#define EVEN_LONGER_NAME (2)
#define foo(x) (x * x)
#define bar(y, z) (y + z)