Skip to content

Commit

Permalink
Merge pull request #71 from SC-SGS/format
Browse files Browse the repository at this point in the history
Add new auto formatting targets (run on PR and pushes to main).
  • Loading branch information
breyerml authored Nov 12, 2024
2 parents 0d3387c + 0862cec commit 5d1991a
Show file tree
Hide file tree
Showing 32 changed files with 140 additions and 67 deletions.
32 changes: 32 additions & 0 deletions .github/workflows/clang_format.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
name: Formatting via clang-format

# only trigger this action on specific events
on:
push:
branches:
- main
pull_request:

jobs:
format:
runs-on: ubuntu-24.04
steps:
# checkout repository
- name: Checkout PLSSVM
uses: actions/[email protected]
with:
path: PLSSVM
# install dependencies
- name: Dependencies
run: |
sudo apt install libomp-dev clang-format
# configure project via CMake
- name: Configure
run: |
cd PLSSVM
cmake --preset all -DPLSSVM_TARGET_PLATFORMS=cpu -DPLSSVM_ENABLE_FORMATTING=ON
# build project
- name: Generate
run: |
cd PLSSVM
cmake --build --preset all --target check-clang-format
29 changes: 29 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -627,6 +627,35 @@ target_link_libraries(${PLSSVM_BASE_LIBRARY_NAME} PUBLIC fmt::fmt)

list(POP_BACK CMAKE_MESSAGE_INDENT)

########################################################################################################################
## enable automatic formatting using clang-format ##
########################################################################################################################
option(PLSSVM_ENABLE_FORMATTING "Enable a formatting targets using clang-format." OFF)
if (PLSSVM_ENABLE_FORMATTING)
list(APPEND CMAKE_MESSAGE_INDENT "Formatting: ")

## install library to add a clang-format target
set(PLSSVM_format_VERSION 1b43a3989de33e44e2a4f1766e9842be1744eef6)
find_package(format QUIET)
if (format_FOUND)
message(STATUS "Found package format.")
else ()
message(STATUS "Couldn't find package format. Building version ${PLSSVM_format_VERSION} from source.")
set(FORMAT_SKIP_CMAKE ON CACHE INTERNAL "" FORCE)
set(FORMAT_SKIP_CLANG OFF CACHE INTERNAL "" FORCE)
# fetch clang-format library
# TODO: use original library after PR (HIP support) has been merged
FetchContent_Declare(format
GIT_REPOSITORY https://github.com/breyerml/Format.cmake.git
GIT_TAG ${PLSSVM_format_VERSION}
QUIET
)
FetchContent_MakeAvailable(format)
endif ()

list(POP_BACK CMAKE_MESSAGE_INDENT)
endif ()

########################################################################################################################
## enable documentation generation via doxygen ##
########################################################################################################################
Expand Down
19 changes: 19 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ General dependencies:
- [doxygen](https://www.doxygen.nl/index.html) if documentation generation is enabled
- [Pybind11 ≥ v2.13.3](https://github.com/pybind/pybind11) if Python bindings are enabled
- [OpenMP](https://www.openmp.org/) 4.0 or newer (optional) to speed-up library utilities (like file parsing)
- [Format.cmake](https://github.com/TheLartians/Format.cmake) if auto formatting via clang-format is enabled; also requires at least clang-format-18 and git
- multiple Python modules used in the utility scripts, to install all modules use `pip install --user -r install/python_requirements.txt`

Additional dependencies for the OpenMP backend:
Expand Down Expand Up @@ -276,6 +277,7 @@ The `[optional_options]` can be one or multiple of:
- `PLSSVM_ENABLE_TESTING=ON|OFF` (default: `ON`): enable testing using GoogleTest and ctest
- `PLSSVM_ENABLE_LANGUAGE_BINDINGS=ON|OFF` (default: `OFF`): enable language bindings
- `PLSSVM_STL_DEBUG_MODE_FLAGS=ON|OFF` (default: `OFF`): enable STL debug modes (**note**: changes the resulting library's ABI!)
- `PLSSVM_ENABLE_FORMATTING=ON|OFF` (default: `OFF`): enable automatic formatting using clang-format; adds additional targets `check-clang-format`, `clang-format`, and `fix-clang-format`

If `PLSSVM_ENABLE_TESTING` is set to `ON`, the following option can also be set:

Expand Down Expand Up @@ -432,6 +434,23 @@ cmake --build . -- coverage

The resulting `html` coverage report is located in the `coverage` folder in the build directory.

### Automatic Source File Formatting

To enable automatic formatting `PLSSVM_ENABLE_FORMATTING` must be set to `ON` and a `clang-format` and `git` executables must be available in `PATH` (minimum `clang-format` version is 18).

To check whether formatting changes must be applied use:

```bash
cmake --build . --target check-clang-format
```

To auto format all files use:

```bash
cmake --build . --target clang-format
cmake --build . --target fix-clang-format
```

### Creating the Documentation

If doxygen is installed and `PLSSVM_ENABLE_DOCUMENTATION` is set to `ON` the documentation can be build using
Expand Down
4 changes: 2 additions & 2 deletions bindings/Python/detail/tracking/performance_tracker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@

#include "plssvm/detail/tracking/performance_tracker.hpp" // plssvm::detail::tracking::{global_tracker, tracking_entry}

#include "plssvm/detail/tracking/events.hpp" // plssvm::detail::tracking::events
#include "plssvm/parameter.hpp" // plssvm::parameter
#include "plssvm/detail/tracking/events.hpp" // plssvm::detail::tracking::events
#include "plssvm/parameter.hpp" // plssvm::parameter

#include "pybind11/chrono.h" // automatic bindings for std::chrono::milliseconds
#include "pybind11/pybind11.h" // py::module_
Expand Down
8 changes: 4 additions & 4 deletions bindings/Python/sklearn.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,10 @@ void parse_provided_params(svc &self, const py::kwargs &args) {
kernel = plssvm::kernel_function_type::rbf;
} else if (kernel_str == "sigmoid") {
kernel = plssvm::kernel_function_type::sigmoid;
} else if (kernel_str == "laplacian") {
kernel = plssvm::kernel_function_type::laplacian;
} else if (kernel_str == "chi_squared") {
kernel = plssvm::kernel_function_type::chi_squared;
} else if (kernel_str == "laplacian") {
kernel = plssvm::kernel_function_type::laplacian;
} else if (kernel_str == "chi_squared") {
kernel = plssvm::kernel_function_type::chi_squared;
} else if (kernel_str == "precomputed") {
throw py::attribute_error{ R"(The "kernel = 'precomputed'" parameter for a call to the 'SVC' constructor is not implemented yet!)" };
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,4 @@ __global__ void fill_array(value_type *data, const value_type value, const size_

} // namespace plssvm::hip::detail

#endif // PLSSVM_BACKENDS_HIP_DETAIL_FILL_KERNEL_HPP_
#endif // PLSSVM_BACKENDS_HIP_DETAIL_FILL_KERNEL_HPP_
2 changes: 1 addition & 1 deletion include/plssvm/backends/OpenCL/detail/pinned_memory.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,6 @@ class [[nodiscard]] pinned_memory final : public ::plssvm::detail::host_pinned_m
extern template class pinned_memory<float>;
extern template class pinned_memory<double>;

} // namespace plssvm::adaptivecpp::detail
} // namespace plssvm::opencl::detail

#endif // PLSSVM_BACKENDS_OPENCL_DETAIL_PINNED_MEMORY_HPP_
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,6 @@ class [[nodiscard]] pinned_memory final : public ::plssvm::detail::host_pinned_m
extern template class pinned_memory<float>;
extern template class pinned_memory<double>;

} // namespace plssvm::adaptivecpp::detail
} // namespace plssvm::dpcpp::detail

#endif // PLSSVM_BACKENDS_SYCL_DPCPP_DETAIL_PINNED_MEMORY_HPP_
1 change: 0 additions & 1 deletion include/plssvm/backends/execution_range.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,6 @@ struct execution_range {
/// The grids. Multiple grids are used, if the grid sizes would exceed the maximum allowed number. Also stores the offsets for the respective grids used in the kernels.
/// Note: no default initialization due to a linker error occurring with NVIDIA's nvhpc!
std::vector<grid_type> grids;

};

/**
Expand Down
3 changes: 1 addition & 2 deletions include/plssvm/backends/gpu_device_ptr.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ template <typename T, typename queue_t, typename device_pointer_t, typename deri
class gpu_device_ptr {
// make sure only valid template types are used
static_assert(detail::tuple_contains_v<T, detail::supported_real_types>,
"Illegal real type provided! See the 'real_type_list' in the type_list.hpp header for a list of the allowed types.");
"Illegal real type provided! See the 'real_type_list' in the type_list.hpp header for a list of the allowed types.");

public:
/// The type of the values used in the device_ptr.
Expand Down Expand Up @@ -370,7 +370,6 @@ class gpu_device_ptr {
device_pointer_type data_{};
};


template <typename T, typename queue_t, typename device_pointer_t, typename derived_gpu_device_ptr>
gpu_device_ptr<T, queue_t, device_pointer_t, derived_gpu_device_ptr>::gpu_device_ptr(const size_type size, const queue_type queue) :
queue_{ queue },
Expand Down
2 changes: 1 addition & 1 deletion include/plssvm/backends/stdpar/csvm.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ class csvm : public ::plssvm::csvm {
*/
[[nodiscard]] implementation_type get_implementation_type() const noexcept;

protected:
protected:
/**
* @copydoc plssvm::csvm::get_device_memory
*/
Expand Down
4 changes: 2 additions & 2 deletions include/plssvm/backends/stdpar/kernel/kernel_functions.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@
#include "plssvm/constants.hpp" // plssvm::real_type
#include "plssvm/kernel_function_types.hpp" // plssvm::kernel_function_type

#if defined(PLSSVM_STDPAR_BACKEND_HAS_INTEL_LLVM) // TODO: remove after linker error is fixed on AMD GPUs
#include "sycl/sycl.hpp" // override std::* math functions
#if defined(PLSSVM_STDPAR_BACKEND_HAS_INTEL_LLVM) // TODO: remove after linker error is fixed on AMD GPUs
#include "sycl/sycl.hpp" // override std::* math functions
#endif

#if defined(PLSSVM_STDPAR_BACKEND_HAS_HIPSTDPAR)
Expand Down
4 changes: 2 additions & 2 deletions include/plssvm/csvm_factory.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ template <typename... Args>
* @return the C-SVM (`[[nodiscard]]`)
*/
template <typename... Args>
[[nodiscard]] inline std::unique_ptr<csvm> make_csvm(const backend_type backend, Args ...args) {
[[nodiscard]] inline std::unique_ptr<csvm> make_csvm(const backend_type backend, Args... args) {
return detail::make_csvm_impl(backend, args...);
}

Expand All @@ -170,7 +170,7 @@ template <typename... Args>
* @return the C-SVM (`[[nodiscard]]`)
*/
template <typename... Args>
[[nodiscard]] inline std::unique_ptr<csvm> make_csvm(Args ...args) {
[[nodiscard]] inline std::unique_ptr<csvm> make_csvm(Args... args) {
return detail::make_csvm_impl(backend_type::automatic, args...);
}

Expand Down
2 changes: 1 addition & 1 deletion include/plssvm/detail/io/scaling_factors_parsing.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
#include "plssvm/exceptions/exceptions.hpp" // plssvm::invalid_file_format_exception

#include "fmt/format.h" // fmt::format
#include "fmt/os.h" // fmt::ostream, fmt::output_file
#include "fmt/os.h" // fmt::ostream, fmt::output_file

#include <exception> // std::exception_ptr, std::exception, std::current_exception, std::rethrow_exception
#include <string> // std::string
Expand Down
2 changes: 1 addition & 1 deletion src/main_scale.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
#include "plssvm/detail/utility.hpp" // PLSSVM_IS_DEFINED

#if defined(PLSSVM_HARDWARE_SAMPLING_ENABLED)
#include "hws/system_hardware_sampler.hpp" // hws::system_hardware_sampler
#include "hws/system_hardware_sampler.hpp" // hws::system_hardware_sampler
#endif

#include <algorithm> // std::for_each
Expand Down
4 changes: 2 additions & 2 deletions src/plssvm/backends/OpenCL/detail/command_queue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@

#include "CL/cl.h" // cl_context, cl_command_queue, cl_device_id, clCreateCommandQueueWithProperties, clCreateCommandQueue, clReleaseCommandQueue

#include <exception> // std::terminate
#include <iostream> // std::cerr, std::endl
#include <exception> // std::terminate
#include <iostream> // std::cerr, std::endl
#include <memory> // std::addressof
#include <type_traits> // std::is_same_v
#include <utility> // std::exchange, std::move
Expand Down
2 changes: 1 addition & 1 deletion src/plssvm/backends/OpenCL/detail/context.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ context::context(context &&other) noexcept :
platform{ std::exchange(other.platform, nullptr) },
devices{ std::move(other.devices) } { }

context &context::operator=(context &&other)noexcept {
context &context::operator=(context &&other) noexcept {
if (this != std::addressof(other)) {
other.device_context = std::exchange(other.device_context, nullptr);
platform = std::exchange(other.platform, nullptr);
Expand Down
6 changes: 3 additions & 3 deletions src/plssvm/backends/stdpar/detail/utility.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,16 +51,16 @@ std::string get_stdpar_version() {
#if defined(PLSSVM_STDPAR_BACKEND_HAS_ACPP)
return plssvm::adaptivecpp::detail::get_adaptivecpp_version_short();
#elif defined(PLSSVM_STDPAR_BACKEND_HAS_NVHPC)
#if defined(PLSSVM_STDPAR_BACKEND_NVHPC_GPU)
#if defined(PLSSVM_STDPAR_BACKEND_NVHPC_GPU)
int runtime_version{};
cudaRuntimeGetVersion(&runtime_version);
// parse it to a more useful string
int major_version = runtime_version / 1000;
int minor_version = runtime_version % 1000 / 10;
return fmt::format("{}.{}.{}; {}.{}", __NVCOMPILER_MAJOR__, __NVCOMPILER_MINOR__, __NVCOMPILER_PATCHLEVEL__, major_version, minor_version);
#else
#else
return fmt::format("{}.{}.{}", __NVCOMPILER_MAJOR__, __NVCOMPILER_MINOR__, __NVCOMPILER_PATCHLEVEL__);
#endif
#endif
#elif defined(PLSSVM_STDPAR_BACKEND_HAS_INTEL_LLVM)
return fmt::format("{}", __VERSION__);
#elif defined(PLSSVM_STDPAR_BACKEND_HAS_GNU_TBB)
Expand Down
4 changes: 2 additions & 2 deletions src/plssvm/detail/cmd/parser_predict.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
#include "plssvm/verbosity_levels.hpp" // plssvm::verbosity, plssvm::verbosity_level
#include "plssvm/version/version.hpp" // plssvm::version::detail::get_version_info

#include "cxxopts.hpp" // cxxopts::{Options, value, ParseResult}
#include "fmt/color.h" // fmt::fg, fmt::color::orange
#include "cxxopts.hpp" // cxxopts::{Options, value, ParseResult}
#include "fmt/color.h" // fmt::fg, fmt::color::orange
#include "fmt/format.h" // fmt::format
#include "fmt/ranges.h" // fmt::join

Expand Down
4 changes: 2 additions & 2 deletions src/plssvm/detail/cmd/parser_scale.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@
#include "plssvm/verbosity_levels.hpp" // plssvm::verbosity, plssvm::verbosity_level
#include "plssvm/version/version.hpp" // plssvm::version::detail::get_version_info

#include "cxxopts.hpp" // cxxopts::{Options, value, ParseResult}
#include "fmt/color.h" // fmt::fg, fmt::color::red
#include "cxxopts.hpp" // cxxopts::{Options, value, ParseResult}
#include "fmt/color.h" // fmt::fg, fmt::color::red
#include "fmt/format.h" // fmt::format
#include "fmt/ranges.h" // fmt::join

Expand Down
2 changes: 1 addition & 1 deletion tests/backends/CUDA/cuda_csvm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -159,4 +159,4 @@ using cuda_mock_kernel_function_type_gtest = util::combine_test_parameters_gtest

// generic GPU CSVM tests - mocked grid sizes
INSTANTIATE_TYPED_TEST_SUITE_P(CUDACSVMFakedGridSize, GenericGPUCSVM, cuda_mock_csvm_test_type_gtest, naming::test_parameter_to_name);
INSTANTIATE_TYPED_TEST_SUITE_P(CUDACSVMFakedGridSize, GenericGPUCSVMKernelFunction, cuda_mock_kernel_function_type_gtest, naming::test_parameter_to_name);
INSTANTIATE_TYPED_TEST_SUITE_P(CUDACSVMFakedGridSize, GenericGPUCSVMKernelFunction, cuda_mock_kernel_function_type_gtest, naming::test_parameter_to_name);
2 changes: 1 addition & 1 deletion tests/backends/OpenCL/opencl_csvm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -157,4 +157,4 @@ using opencl_mock_kernel_function_type_gtest = util::combine_test_parameters_gte

// generic GPU CSVM tests - mocked grid sizes
INSTANTIATE_TYPED_TEST_SUITE_P(OpenCLCSVMFakedGridSize, GenericGPUCSVM, opencl_mock_csvm_test_type_gtest, naming::test_parameter_to_name);
INSTANTIATE_TYPED_TEST_SUITE_P(OpenCLCSVMFakedGridSize, GenericGPUCSVMKernelFunction, opencl_mock_kernel_function_type_gtest, naming::test_parameter_to_name);
INSTANTIATE_TYPED_TEST_SUITE_P(OpenCLCSVMFakedGridSize, GenericGPUCSVMKernelFunction, opencl_mock_kernel_function_type_gtest, naming::test_parameter_to_name);
3 changes: 1 addition & 2 deletions tests/backends/SYCL/DPCPP/mock_dpcpp_csvm.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,14 @@
#define PLSSVM_TESTS_BACKENDS_SYCL_DPCPP_MOCK_DPCPP_CSVM_HPP_
#pragma once

#include "plssvm/backends/SYCL/DPCPP/csvm.hpp" // plssvm::dpcpp::csvm
#include "plssvm/backends/execution_range.hpp" // plssvm::detail::dim_type
#include "plssvm/backends/SYCL/DPCPP/csvm.hpp" // plssvm::dpcpp::csvm

#include "gmock/gmock.h" // MOCK_METHOD, ON_CALL, ::testing::Return

#include <cstddef> // std::size_t
#include <utility> // std::forward


/**
* @brief GTest mock class for the SYCL CSVM using DPC++ as SYCL implementation.
* @tparam mock_grid_size `true` if the `plssvm::dpcpp::csvm::get_max_grid_size()` function should be mocked, otherwise `false`
Expand Down
12 changes: 5 additions & 7 deletions tests/backends/ground_truth.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@
#include "plssvm/parameter.hpp" // plssvm::parameter
#include "plssvm/shape.hpp" // plssvm::shape

#include <cmath> // std::pow, std::exp, std::fma
#include <cstddef> // std::size_t
#include <utility> // std::pair, std::make_pair, std::move
#include <variant> // std::get
#include <vector> // std::vector
#include <cmath> // std::pow, std::exp, std::fma
#include <cstddef> // std::size_t
#include <utility> // std::pair, std::make_pair, std::move
#include <variant> // std::get
#include <vector> // std::vector

namespace ground_truth {

Expand Down Expand Up @@ -116,7 +116,6 @@ real_type chi_squared_kernel(const std::vector<real_type> &x, const std::vector<
template float chi_squared_kernel(const std::vector<float> &, const std::vector<float> &, const float);
template double chi_squared_kernel(const std::vector<double> &, const std::vector<double> &, const double);


template <typename real_type, plssvm::layout_type layout>
real_type linear_kernel(const plssvm::matrix<real_type, layout> &X, const std::size_t i, const plssvm::matrix<real_type, layout> &Y, const std::size_t j) {
PLSSVM_ASSERT(X.num_cols() == Y.num_cols(), "Sizes mismatch!: {} != {}", X.num_cols(), Y.num_cols());
Expand Down Expand Up @@ -364,7 +363,6 @@ plssvm::aos_matrix<real_type> predict_values(const plssvm::parameter &params, co
template plssvm::aos_matrix<float> predict_values(const plssvm::parameter &, const plssvm::soa_matrix<float> &, const plssvm::aos_matrix<float> &, const std::vector<float> &, const plssvm::soa_matrix<float> &, const plssvm::soa_matrix<float> &, const std::size_t, const std::size_t);
template plssvm::aos_matrix<double> predict_values(const plssvm::parameter &, const plssvm::soa_matrix<double> &, const plssvm::aos_matrix<double> &, const std::vector<double> &, const plssvm::soa_matrix<double> &, const plssvm::soa_matrix<double> &, const std::size_t, const std::size_t);


} // namespace detail

template <typename real_type>
Expand Down
1 change: 0 additions & 1 deletion tests/backends/ground_truth.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,6 @@ template <typename real_type, plssvm::layout_type layout>
template <typename real_type>
[[nodiscard]] plssvm::aos_matrix<real_type> predict_values(const plssvm::parameter &params, const plssvm::soa_matrix<real_type> &w, const plssvm::aos_matrix<real_type> &weights, const std::vector<real_type> &rho, const plssvm::soa_matrix<real_type> &support_vectors, const plssvm::soa_matrix<real_type> &predict_points, std::size_t row_offset, std::size_t device_specific_num_rows);


} // namespace detail

/**
Expand Down
14 changes: 7 additions & 7 deletions tests/backends/stdpar/implementation_types.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +71,11 @@ TEST(stdparImplementationType, from_string_unknown) {
TEST(stdparImplementationType, minimal_available_stdpar_implementation_type) {
const std::vector<plssvm::stdpar::implementation_type> implementation_type = plssvm::stdpar::list_available_stdpar_implementations();

#if defined(PLSSVM_HAS_STDPAR_BACKEND)
// at least one must be available!
EXPECT_GE(implementation_type.size(), 1);
#else
// stdpar not active -> no implementation type available
EXPECT_TRUE(implementation_type.empty());
#endif
#if defined(PLSSVM_HAS_STDPAR_BACKEND)
// at least one must be available!
EXPECT_GE(implementation_type.size(), 1);
#else
// stdpar not active -> no implementation type available
EXPECT_TRUE(implementation_type.empty());
#endif
}
Loading

0 comments on commit 5d1991a

Please sign in to comment.