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

Apply formatting for C++ using clang-format #2652

Merged
merged 11 commits into from
Aug 24, 2023
  •  
  •  
  •  
6 changes: 6 additions & 0 deletions .clang-format
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Language: Cpp
BasedOnStyle: LLVM
BinPackArguments: false
BinPackParameters: false
PackConstructorInitializers: NextLine
ColumnLimit: 120
5 changes: 5 additions & 0 deletions .git-blame-ignore-revs
Original file line number Diff line number Diff line change
Expand Up @@ -97,3 +97,8 @@ aad44d847ac48d02bb7f8badf801dbfaa0ccdac0
#Author: MRtrixBot <gitbot@mrtrix.org>
#Date: Tue Jan 3 13:42:58 2023 +0100
# Update Copyright notice in command docs

45fc06bdf687584bf4d55140720e84c5ef517bf0
#Author: MRtrixBot <gitbot@mrtrix.org>
#Date: Thu, 17 Aug 2023 15:36:14 +0100
# Apply formatting for C++ using clang-format
13 changes: 13 additions & 0 deletions .github/workflows/checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -214,3 +214,16 @@ jobs:

- name: check building of documentation
run: python3 -m sphinx -n -N -W -w sphinx.log docs/ tmp/

clang-format-check:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v1

- name: clang-format
uses: jidicula/clang-format-action@v4.11.0
with:
clang-format-version: '16'
fallback-style: 'LLVM'
# Ignore third-party headers.
exclude-regex: 'core/file/(json|nifti[12]).h'
10 changes: 10 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
default_language_version:
python: python3
repos:
- repo: https://github.com/pre-commit/mirrors-clang-format
rev: v16.0.4
hooks:
- id: clang-format
name: clang-format
types_or: [c++]
language: python
27 changes: 27 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,33 @@ A few explicit notes on such:
introduce Windows-style newline characters ("`CR LF`" / "`\r\n`")
will need to be edited accordingly.

- In C++, we use [clang-format](https://clang.llvm.org/docs/ClangFormat.html) 16
to ensure that all C++ code is formatted using the same conventions.
To comply with this requirement, we recommend that you install a local git hook
using [pre-commit](https://pre-commit.com/):

- Install `pre-commit` via `pip install pre-commit` (or via your OS package manager).

- Run `pre-commit install` in the source directory of MRtrix.
NOTE: you may need to add the pip scripts folder to your `PATH`
(you can check where pip has installed the package using `pip show pre-commit`).

This procedure will install a local git hook to ensure that all of your commits
are correctly formatted. If this isn't the case, the commits will fail and an
automatic formatting of the staged changes will be applied. You will then have
to commit those changes again.

Alternatively, you may wish to configure your editor to automatically format
your code using `clang-format`. We also provide a Python script in the top level directory
called `clang-format-all` that can format all C++ code in the repository. You can run
this to format your code before committing it. The script also allows you to
specify a custom `clang-format` binary by using the `--executable` flag.

NOTE: If you are using a version of `clang-format` that is different from the one
used by the CI (currently version 16), your code may be formatted differently and thus
fail our CI tests. We recommend that you use the same version of `clang-format` as the
CI to avoid this issue.

- In Python, variable / class / module names are enforced through
`pylint`. Script "`run_pylint`" in the *MRtrix3* root directory
will test any code modifications against these expectations.
Expand Down
85 changes: 85 additions & 0 deletions clang-format-all
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
#!/usr/env/python3

# Copyright (c) 2008-2023 the MRtrix3 contributors.
#
# This Source Code Form is subject to the terms of the Mozilla Public
# License, v. 2.0. If a copy of the MPL was not distributed with this
# file, You can obtain one at http://mozilla.org/MPL/2.0/.
#
# Covered Software is provided under this License on an "as is"
# basis, without warranty of any kind, either expressed, implied, or
# statutory, including, without limitation, warranties that the
# Covered Software is free of defects, merchantable, fit for a
# particular purpose or non-infringing.
# See the Mozilla Public License v. 2.0 for more details.
#
# For more details, see http://www.mrtrix.org/.

import os
import argparse
import subprocess
import time
from pathlib import Path
from multiprocessing import cpu_count

parser = argparse.ArgumentParser(
description='Run clang-format on C++ source files.'
)

parser.add_argument(
'-e', '--executable',
help='Path to the clang-format executable.',
default='clang-format'
)

args = parser.parse_args()
clang_format = args.executable
paths = ['core', 'cmd', 'src', 'testing']
extensions = ['.h', '.cpp']
exclusion_list = ['core/file/nifti1.h',
'core/file/nifti2.h',
'core/file/json.h']

# if clang-format path contains spaces, wrap it in quotes
if ' ' in clang_format and not clang_format.startswith('"'):
clang_format = '"{}"'.format(clang_format)

if os.system('{} -version'.format(clang_format)) != 0:
raise RuntimeError('Could not find clang-format executable.')


def skip_file(file):
"""Return True if the file should be skipped."""
return file.suffix not in extensions or file.as_posix() in exclusion_list


def format_file(file):
"""Run clang-format on a file."""
command = '{} -i {}'.format(clang_format, file)
return subprocess.Popen(command, shell=True)


files = []
for path in paths:
files += [file for file in list(Path(path).rglob('*.*'))
if not skip_file(file)]

print('Found {} files to format.'.format(len(files)))

# We want a maximum of num_cpus processes running at once
processes = []
num_cpus = cpu_count()
total = len(files)

try:
while len(files) > 0 or len(processes) > 0:
processes = [proc for proc in processes if proc.poll() is None]
schedule_count = min(num_cpus - len(processes), len(files))
processes += [format_file(files.pop(0)) for _ in range(schedule_count)]
print('Formatted {}/{} files.'.format(total - len(files), total), end='\r')
time.sleep(0.01)

except KeyboardInterrupt:
print('Keyboard interrupt received, terminating formatting.')
for process in processes:
process.terminate()
145 changes: 66 additions & 79 deletions cmd/5tt2gmwmi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,110 +25,100 @@
#include "dwi/tractography/ACT/act.h"
#include "dwi/tractography/ACT/tissues.h"



using namespace MR;
using namespace App;


void usage ()
{
void usage() {

AUTHOR = "Robert E. Smith (robert.smith@florey.edu.au)";

SYNOPSIS = "Generate a mask image appropriate for seeding streamlines on the grey matter-white matter interface";

REFERENCES
+ "Smith, R. E.; Tournier, J.-D.; Calamante, F. & Connelly, A. " // Internal
"Anatomically-constrained tractography:"
"Improved diffusion MRI streamlines tractography through effective use of anatomical information. "
"NeuroImage, 2012, 62, 1924-1938";
+"Smith, R. E.; Tournier, J.-D.; Calamante, F. & Connelly, A. " // Internal
"Anatomically-constrained tractography:"
"Improved diffusion MRI streamlines tractography through effective use of anatomical information. "
"NeuroImage, 2012, 62, 1924-1938";

ARGUMENTS
+ Argument ("5tt_in", "the input 5TT segmented anatomical image").type_image_in()
+ Argument ("mask_out", "the output mask image") .type_image_out();
+Argument("5tt_in", "the input 5TT segmented anatomical image").type_image_in() +
Argument("mask_out", "the output mask image").type_image_out();

OPTIONS
+ Option("mask_in", "Filter an input mask image according to those voxels that lie upon the grey matter - white matter boundary. "
"If no input mask is provided, the output will be a whole-brain mask image calculated using the anatomical image only.")
+ Argument ("image", "the input mask image").type_image_in();

+Option("mask_in",
"Filter an input mask image according to those voxels that lie upon the grey matter - white matter boundary. "
"If no input mask is provided, the output will be a whole-brain mask image calculated using the anatomical "
"image only.") +
Argument("image", "the input mask image").type_image_in();
}

class Processor {

public:
Processor(const Image<bool> &mask) : mask(mask) {}
Processor(const Processor &) = default;

class Processor
{

public:
Processor (const Image<bool>& mask) : mask (mask) { }
Processor (const Processor&) = default;

bool operator() (Image<float>& input, Image<float>& output)
{
// If a mask is defined, but is false in this voxel, do not continue processing
bool process_voxel = true;
if (mask.valid()) {
assign_pos_of (input, 0, 3).to (mask);
process_voxel = mask.value();
}
if (process_voxel) {
// Generate a non-binary seeding mask.
// Image intensity should be proportional to the tissue gradient present across the voxel
// Remember: This seeding mask is generated in the same space as the 5TT image: exploit this
// (no interpolators should be necessary)
// Essentially looking for an absolute gradient in (GM - WM) - just do in three axes
// - well, not quite; needs to be the minimum of the two
default_type gradient = 0.0;
for (size_t axis = 0; axis != 3; ++axis) {
assign_pos_of (output, 0, 3).to (input);
default_type multiplier = 0.5;
if (!output.index(axis)) {
multiplier = 1.0;
} else {
input.move_index (axis, -1);
}
const DWI::Tractography::ACT::Tissues neg (input);
if (output.index(axis) == output.size(axis)-1) {
multiplier = 1.0;
input.index(axis) = output.index(axis);
} else {
input.index(axis) = output.index(axis) + 1;
}
const DWI::Tractography::ACT::Tissues pos (input);
gradient += Math::pow2 (multiplier * std::min (abs (pos.get_gm() - neg.get_gm()), abs (pos.get_wm() - neg.get_wm())));
bool operator()(Image<float> &input, Image<float> &output) {
// If a mask is defined, but is false in this voxel, do not continue processing
bool process_voxel = true;
if (mask.valid()) {
assign_pos_of(input, 0, 3).to(mask);
process_voxel = mask.value();
}
if (process_voxel) {
// Generate a non-binary seeding mask.
// Image intensity should be proportional to the tissue gradient present across the voxel
// Remember: This seeding mask is generated in the same space as the 5TT image: exploit this
// (no interpolators should be necessary)
// Essentially looking for an absolute gradient in (GM - WM) - just do in three axes
// - well, not quite; needs to be the minimum of the two
default_type gradient = 0.0;
for (size_t axis = 0; axis != 3; ++axis) {
assign_pos_of(output, 0, 3).to(input);
default_type multiplier = 0.5;
if (!output.index(axis)) {
multiplier = 1.0;
} else {
input.move_index(axis, -1);
}
output.value() = std::max (0.0, std::sqrt (gradient));
assign_pos_of (output, 0, 3).to (input);
} else {
output.value() = 0.0f;
const DWI::Tractography::ACT::Tissues neg(input);
if (output.index(axis) == output.size(axis) - 1) {
multiplier = 1.0;
input.index(axis) = output.index(axis);
} else {
input.index(axis) = output.index(axis) + 1;
}
const DWI::Tractography::ACT::Tissues pos(input);
gradient +=
Math::pow2(multiplier * std::min(abs(pos.get_gm() - neg.get_gm()), abs(pos.get_wm() - neg.get_wm())));
}
return true;
output.value() = std::max(0.0, std::sqrt(gradient));
assign_pos_of(output, 0, 3).to(input);
} else {
output.value() = 0.0f;
}
return true;
}

private:
Image<bool> mask;

private:
Image<bool> mask;
};

void run() {


void run ()
{

auto input = Image<float>::open (argument[0]);
DWI::Tractography::ACT::verify_5TT_image (input);
check_3D_nonunity (input);
auto input = Image<float>::open(argument[0]);
DWI::Tractography::ACT::verify_5TT_image(input);
check_3D_nonunity(input);

// TODO It would be nice to have the capability to define this mask based on another image
// This will however require the use of interpolators

Image<bool> mask;
auto opt = get_options ("mask_in");
auto opt = get_options("mask_in");
if (opt.size()) {
mask = Image<bool>::open (opt[0][0]);
if (!dimensions_match (input, mask, 0, 3))
throw Exception ("Mask image provided using the -mask option must match the input 5TT image");
mask = Image<bool>::open(opt[0][0]);
if (!dimensions_match(input, mask, 0, 3))
throw Exception("Mask image provided using the -mask option must match the input 5TT image");
}

Header H;
Expand All @@ -140,10 +130,7 @@ void run ()
H = input;
H.ndim() = 3;
}
auto output = Image<float>::create (argument[1], H);

ThreadedLoop ("Generating GMWMI seed mask", input, 0, 3)
.run (Processor (mask), input, output);
auto output = Image<float>::create(argument[1], H);

ThreadedLoop("Generating GMWMI seed mask", input, 0, 3).run(Processor(mask), input, output);
}

Loading