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.

Sign up for GitHub

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

Cleaning up some warnings and enabling -Werror on CPU CI tests #107

Merged
merged 9 commits into from
Jul 2, 2024
Merged

Cleaning up some warnings and enabling -Werror on CPU CI tests #107

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
d4ccf93
Fix compilation warnings
cedricchevalier19 Jun 26, 2024
393b52d
Fix compilation warnings
cedricchevalier19 Jun 26, 2024
cc5ff43
Add warnings on CPU CI
cedricchevalier19 Jun 26, 2024
59f0dd6
Fix format
cedricchevalier19 Jun 26, 2024
bbba18d
Fix format
cedricchevalier19 Jun 26, 2024
7f1bbee
Fix format with correct clang-format version
cedricchevalier19 Jun 27, 2024
300a339
Reintroducing some ExecutionSpace
cedricchevalier19 Jul 2, 2024
4f31190
Better comments
cedricchevalier19 Jul 2, 2024
b56e2de
Remove unneeded comment
cedricchevalier19 Jul 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions .github/workflows/build_test.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,23 +73,23 @@ jobs:
cxx: g++
cmake_flags:
kokkos: -DKokkos_ENABLE_OPENMP=ON
kokkos_fft: ""
kokkos_fft: -DCMAKE_CXX_FLAGS="-Wall -Wextra" -DCMAKE_COMPILE_WARNING_AS_ERROR=ON
- name: threads
image: gcc
compiler:
c: gcc
cxx: g++
cmake_flags:
kokkos: -DKokkos_ENABLE_THREADS=ON
kokkos_fft: ""
kokkos_fft: -DCMAKE_CXX_FLAGS="-Wall -Wextra" -DCMAKE_COMPILE_WARNING_AS_ERROR=ON
- name: serial
image: gcc
compiler:
c: gcc
cxx: g++
cmake_flags:
kokkos: -DKokkos_ENABLE_SERIAL=ON
kokkos_fft: ""
kokkos_fft: -DCMAKE_CXX_FLAGS="-Wall -Wextra" -DCMAKE_COMPILE_WARNING_AS_ERROR=ON
- name: cuda
image: nvcc
compiler:
Expand Down
15 changes: 8 additions & 7 deletions common/src/KokkosFFT_Helpers.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ auto _get_shift(const ViewType& inout, axis_type<DIM> _axes,
assert(!KokkosFFT::Impl::is_out_of_range_value_included(axes, rank));

axis_type<rank> shift = {0};
for (int i = 0; i < DIM; i++) {
for (int i = 0; i < static_cast<int>(DIM); i++) {
int axis = axes.at(i);
shift.at(axis) = inout.extent(axis) / 2 * direction;
}
Expand All @@ -40,7 +40,8 @@ auto _get_shift(const ViewType& inout, axis_type<DIM> _axes,

template <typename ExecutionSpace, typename ViewType>
void _roll(const ExecutionSpace& exec_space, ViewType& inout,
axis_type<1> shift, axis_type<1> axes) {
axis_type<1> shift, axis_type<1>) {
// Last parameter is ignored but present for keeping the interface consistent
static_assert(ViewType::rank() == 1, "_roll: Rank of View must be 1.");
std::size_t n0 = inout.extent(0);

Expand All @@ -56,7 +57,7 @@ void _roll(const ExecutionSpace& exec_space, ViewType& inout,
Kokkos::parallel_for(
Kokkos::RangePolicy<ExecutionSpace, Kokkos::IndexType<std::size_t>>(
exec_space, 0, len),
KOKKOS_LAMBDA(const int& i) {
KOKKOS_LAMBDA(std::size_t i) {
tmp(i + shift0) = inout(i);
if (i + shift1 < n0) {
tmp(i) = inout(i + shift1);
Expand All @@ -70,7 +71,7 @@ void _roll(const ExecutionSpace& exec_space, ViewType& inout,
template <typename ExecutionSpace, typename ViewType, std::size_t DIM1 = 1>
void _roll(const ExecutionSpace& exec_space, ViewType& inout,
axis_type<2> shift, axis_type<DIM1> axes) {
constexpr std::size_t DIM0 = 2;
constexpr int DIM0 = 2;
static_assert(ViewType::rank() == DIM0, "_roll: Rank of View must be 2.");
int n0 = inout.extent(0), n1 = inout.extent(1);

Expand All @@ -90,7 +91,7 @@ void _roll(const ExecutionSpace& exec_space, ViewType& inout,
);

axis_type<2> shift0 = {0}, shift1 = {0}, shift2 = {n0 / 2, n1 / 2};
for (int i = 0; i < DIM1; i++) {
for (int i = 0; static_cast<std::size_t>(i) < DIM1; i++) {
int axis = axes.at(i);

auto [_shift0, _shift1, _shift2] =
Expand Down Expand Up @@ -179,7 +180,7 @@ namespace KokkosFFT {
///
/// \return Sampling frequency
template <typename ExecutionSpace, typename RealType>
auto fftfreq(const ExecutionSpace& exec_space, const std::size_t n,
auto fftfreq(const ExecutionSpace&, const std::size_t n,
const RealType d = 1.0) {
static_assert(std::is_floating_point<RealType>::value,
"fftfreq: d must be float or double");
Expand Down Expand Up @@ -214,7 +215,7 @@ auto fftfreq(const ExecutionSpace& exec_space, const std::size_t n,
///
/// \return Sampling frequency starting from zero
template <typename ExecutionSpace, typename RealType>
auto rfftfreq(const ExecutionSpace& exec_space, const std::size_t n,
auto rfftfreq(const ExecutionSpace&, const std::size_t n,
const RealType d = 1.0) {
static_assert(std::is_floating_point<RealType>::value,
"fftfreq: d must be float or double");
Expand Down
4 changes: 2 additions & 2 deletions common/src/KokkosFFT_normalization.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ void _normalize(const ExecutionSpace& exec_space, ViewType& inout,
}

template <typename ViewType>
auto _coefficients(const ViewType& inout, Direction direction,
Normalization normalization, std::size_t fft_size) {
auto _coefficients(ViewType, Direction direction, Normalization normalization,
std::size_t fft_size) {
using value_type =
KokkosFFT::Impl::real_type_t<typename ViewType::non_const_value_type>;
value_type coef = 1;
Expand Down
2 changes: 1 addition & 1 deletion common/src/KokkosFFT_padding.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ auto get_modified_shape(const InViewType in, const OutViewType /* out */,
}

// Update shapes based on newly given shape
for (int i = 0; i < DIM; i++) {
for (int i = 0; i < static_cast<int>(DIM); i++) {
int positive_axis = positive_axes.at(i);
assert(shape.at(i) > 0);
modified_shape.at(positive_axis) = shape.at(i);
Expand Down
24 changes: 7 additions & 17 deletions common/src/KokkosFFT_transpose.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,14 +89,14 @@ auto get_map_axes(const ViewType& view, int axis) {
template <class InViewType, class OutViewType, std::size_t DIMS>
void _prep_transpose_view(InViewType& in, OutViewType& out,
axis_type<DIMS> _map) {
constexpr std::size_t rank = OutViewType::rank();
constexpr int rank = OutViewType::rank();

// Assign a View if not a shallow copy
bool is_out_view_ready = true;
std::array<int, rank> out_extents;
for (int i = 0; i < rank; i++) {
out_extents.at(i) = in.extent(_map.at(i));
if (out_extents.at(i) != out.extent(i)) {
if (static_cast<std::size_t>(out_extents.at(i)) != out.extent(i)) {
is_out_view_ready = false;
}
}
Expand All @@ -113,15 +113,15 @@ void _prep_transpose_view(InViewType& in, OutViewType& out,

template <typename ExecutionSpace, typename InViewType, typename OutViewType,
std::enable_if_t<InViewType::rank() == 1, std::nullptr_t> = nullptr>
void _transpose(const ExecutionSpace& exec_space, InViewType& in,
OutViewType& out, [[maybe_unused]] axis_type<1> _map) {}
void _transpose(const ExecutionSpace&, InViewType&, OutViewType&,
axis_type<1>) {
// FIXME: Comment why empty?
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is true. It may be better to remove this and avoid calling _transpose function in transpose for 1D case?

}

template <typename ExecutionSpace, typename InViewType, typename OutViewType>
void _transpose(const ExecutionSpace& exec_space, InViewType& in,
OutViewType& out, axis_type<2> _map) {
constexpr std::size_t DIM = 2;
constexpr std::size_t rank = InViewType::rank();
using array_layout_type = typename InViewType::array_layout;
constexpr std::size_t DIM = 2;

using range_type = Kokkos::MDRangePolicy<
ExecutionSpace,
Expand All @@ -147,7 +147,6 @@ void _transpose(const ExecutionSpace& exec_space, InViewType& in,
OutViewType& out, axis_type<3> _map) {
constexpr std::size_t DIM = 3;
constexpr std::size_t rank = InViewType::rank();
using array_layout_type = typename InViewType::array_layout;

using range_type = Kokkos::MDRangePolicy<
ExecutionSpace,
Expand Down Expand Up @@ -181,7 +180,6 @@ void _transpose(const ExecutionSpace& exec_space, InViewType& in,
OutViewType& out, axis_type<4> _map) {
constexpr std::size_t DIM = 4;
constexpr std::size_t rank = InViewType::rank();
using array_layout_type = typename InViewType::array_layout;

using range_type = Kokkos::MDRangePolicy<
ExecutionSpace,
Expand Down Expand Up @@ -217,7 +215,6 @@ void _transpose(const ExecutionSpace& exec_space, InViewType& in,
OutViewType& out, axis_type<5> _map) {
constexpr std::size_t DIM = 5;
constexpr std::size_t rank = InViewType::rank();
using array_layout_type = typename InViewType::array_layout;

using range_type = Kokkos::MDRangePolicy<
ExecutionSpace,
Expand Down Expand Up @@ -255,7 +252,6 @@ void _transpose(const ExecutionSpace& exec_space, InViewType& in,
OutViewType& out, axis_type<6> _map) {
constexpr std::size_t DIM = 6;
constexpr std::size_t rank = InViewType::rank();
using array_layout_type = typename InViewType::array_layout;

using range_type = Kokkos::MDRangePolicy<
ExecutionSpace,
Expand Down Expand Up @@ -296,7 +292,6 @@ void _transpose(const ExecutionSpace& exec_space, InViewType& in,
OutViewType& out, axis_type<7> _map) {
constexpr std::size_t DIM = 6;
constexpr std::size_t rank = InViewType::rank();
using array_layout_type = typename InViewType::array_layout;

using range_type = Kokkos::MDRangePolicy<
ExecutionSpace,
Expand Down Expand Up @@ -342,7 +337,6 @@ void _transpose(const ExecutionSpace& exec_space, InViewType& in,
constexpr std::size_t DIM = 6;

constexpr std::size_t rank = InViewType::rank();
using array_layout_type = typename InViewType::array_layout;

using range_type = Kokkos::MDRangePolicy<
ExecutionSpace,
Expand Down Expand Up @@ -409,10 +403,6 @@ template <typename ExecutionSpace, typename InViewType, typename OutViewType,
std::size_t DIM = 1>
void transpose(const ExecutionSpace& exec_space, InViewType& in,
OutViewType& out, axis_type<DIM> _map) {
using in_value_type = typename InViewType::non_const_value_type;
using out_value_type = typename OutViewType::non_const_value_type;
using array_layout_type = typename InViewType::array_layout;

static_assert(Kokkos::is_view<InViewType>::value,
"transpose: InViewType is not a Kokkos::View.");
static_assert(Kokkos::is_view<InViewType>::value,
Expand Down
10 changes: 3 additions & 7 deletions common/src/KokkosFFT_utils.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ struct complex_view_type {
};

template <typename ViewType>
auto convert_negative_axis(const ViewType& view, int _axis = -1) {
auto convert_negative_axis(ViewType, int _axis = -1) {
static_assert(Kokkos::is_view<ViewType>::value,
"convert_negative_axis: ViewType is not a Kokkos::View.");
int rank = static_cast<int>(ViewType::rank());
Expand Down Expand Up @@ -160,7 +160,7 @@ inline std::vector<ElementType> arange(const ElementType start,
ElementType delta = (stop - start) / length;

// thrust::sequence
for (auto i = 0; i < length; i++) {
for (std::size_t i = 0; i < length; i++) {
ElementType value = start + delta * i;
result.push_back(value);
}
Expand All @@ -175,7 +175,6 @@ void conjugate(const ExecutionSpace& exec_space, const InViewType& in,
static_assert(Kokkos::is_view<OutViewType>::value,
"conjugate: OutViewType is not a Kokkos::View.");

using in_value_type = typename InViewType::non_const_value_type;
using out_value_type = typename OutViewType::non_const_value_type;

static_assert(KokkosFFT::Impl::is_complex<out_value_type>::value,
Expand All @@ -189,10 +188,7 @@ void conjugate(const ExecutionSpace& exec_space, const InViewType& in,
Kokkos::parallel_for(
Kokkos::RangePolicy<ExecutionSpace, Kokkos::IndexType<std::size_t>>(
exec_space, 0, size),
KOKKOS_LAMBDA(const int& i) {
out_value_type tmp = in_data[i];
out_data[i] = Kokkos::conj(in_data[i]);
});
KOKKOS_LAMBDA(std::size_t i) { out_data[i] = Kokkos::conj(in_data[i]); });
}

template <typename ViewType>
Expand Down
10 changes: 5 additions & 5 deletions common/unit_test/Test_Helpers.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ void test_fftshift1D_1DView(int n0) {
_y_ref = {-4, -3, -2, -1, 0, 1, 2, 3, 4};
}

for (std::size_t i = 0; i < n0; i++) {
for (int i = 0; i < n0; i++) {
h_x_ref(i) = static_cast<double>(_x_ref.at(i));
h_y_ref(i) = static_cast<double>(_y_ref.at(i));
}
Expand Down Expand Up @@ -283,8 +283,8 @@ void test_fftshift1D_2DView(int n0) {
5, 6, 7, 8, 9, 10, 11, 12, 13, -13, -12, -11, -10};
}

for (std::size_t i1 = 0; i1 < n1; i1++) {
for (std::size_t i0 = 0; i0 < n0; i0++) {
for (int i1 = 0; i1 < n1; i1++) {
for (int i0 = 0; i0 < n0; i0++) {
std::size_t i = i0 + i1 * n0;
h_x_ref(i0, i1) = static_cast<double>(_x_ref.at(i));
h_y_axis0_ref(i0, i1) = static_cast<double>(_y0_ref.at(i));
Expand Down Expand Up @@ -340,8 +340,8 @@ void test_fftshift2D_2DView(int n0) {
1, 2, 3, 4, -13, -12, -11, -10, 9, 10, 11, 12, 13};
}

for (std::size_t i1 = 0; i1 < n1; i1++) {
for (std::size_t i0 = 0; i0 < n0; i0++) {
for (int i1 = 0; i1 < n1; i1++) {
for (int i0 = 0; i0 < n0; i0++) {
std::size_t i = i0 + i1 * n0;
h_x_ref(i0, i1) = static_cast<double>(_x_ref.at(i));
h_y_ref(i0, i1) = static_cast<double>(_y_ref.at(i));
Expand Down
55 changes: 34 additions & 21 deletions common/unit_test/Test_Padding.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1384,8 +1384,9 @@ TEST(CropOrPad3D, 3DView) {
for (int i2 = 0; i2 < n2; i2++) {
for (int i1 = 0; i1 < n1; i1++) {
for (int i0 = 0; i0 < n0; i0++) {
if (i0 >= h_ref_x.extent(0) || i1 >= h_ref_x.extent(1) ||
i2 >= h_ref_x.extent(2))
if (static_cast<std::size_t>(i0) >= h_ref_x.extent(0) ||
static_cast<std::size_t>(i1) >= h_ref_x.extent(1) ||
static_cast<std::size_t>(i2) >= h_ref_x.extent(2))
continue;
h_ref_x(i0, i1, i2) = h_x(i0, i1, i2);
}
Expand Down Expand Up @@ -1426,12 +1427,15 @@ TEST(CropOrPad4D, 4DView) {
View4D<double> ref_x("ref_x", n0_new, n1_new, n2_new, n3_new);

auto h_ref_x = Kokkos::create_mirror_view(ref_x);
// TODO: stop loop early
for (int i3 = 0; i3 < n3; i3++) {
for (int i2 = 0; i2 < n2; i2++) {
for (int i1 = 0; i1 < n1; i1++) {
for (int i0 = 0; i0 < n0; i0++) {
if (i0 >= h_ref_x.extent(0) || i1 >= h_ref_x.extent(1) ||
i2 >= h_ref_x.extent(2) || i3 >= h_ref_x.extent(3))
if (static_cast<std::size_t>(i0) >= h_ref_x.extent(0) ||
static_cast<std::size_t>(i1) >= h_ref_x.extent(1) ||
static_cast<std::size_t>(i2) >= h_ref_x.extent(2) ||
static_cast<std::size_t>(i3) >= h_ref_x.extent(3))
continue;
h_ref_x(i0, i1, i2, i3) = h_x(i0, i1, i2, i3);
}
Expand Down Expand Up @@ -1480,9 +1484,11 @@ TEST(CropOrPad5D, 5DView) {
for (int i2 = 0; i2 < n2; i2++) {
for (int i1 = 0; i1 < n1; i1++) {
for (int i0 = 0; i0 < n0; i0++) {
if (i0 >= h_ref_x.extent(0) || i1 >= h_ref_x.extent(1) ||
i2 >= h_ref_x.extent(2) || i3 >= h_ref_x.extent(3) ||
i4 >= h_ref_x.extent(4))
if (static_cast<std::size_t>(i0) >= h_ref_x.extent(0) ||
static_cast<std::size_t>(i1) >= h_ref_x.extent(1) ||
static_cast<std::size_t>(i2) >= h_ref_x.extent(2) ||
static_cast<std::size_t>(i3) >= h_ref_x.extent(3) ||
static_cast<std::size_t>(i4) >= h_ref_x.extent(4))
continue;
h_ref_x(i0, i1, i2, i3, i4) = h_x(i0, i1, i2, i3, i4);
}
Expand Down Expand Up @@ -1537,9 +1543,12 @@ TEST(CropOrPad6D, 6DView) {
for (int i2 = 0; i2 < n2; i2++) {
for (int i1 = 0; i1 < n1; i1++) {
for (int i0 = 0; i0 < n0; i0++) {
if (i0 >= h_ref_x.extent(0) || i1 >= h_ref_x.extent(1) ||
i2 >= h_ref_x.extent(2) || i3 >= h_ref_x.extent(3) ||
i4 >= h_ref_x.extent(4) || i5 >= h_ref_x.extent(5))
if (static_cast<std::size_t>(i0) >= h_ref_x.extent(0) ||
static_cast<std::size_t>(i1) >= h_ref_x.extent(1) ||
static_cast<std::size_t>(i2) >= h_ref_x.extent(2) ||
static_cast<std::size_t>(i3) >= h_ref_x.extent(3) ||
static_cast<std::size_t>(i4) >= h_ref_x.extent(4) ||
static_cast<std::size_t>(i5) >= h_ref_x.extent(5))
continue;
h_ref_x(i0, i1, i2, i3, i4, i5) =
h_x(i0, i1, i2, i3, i4, i5);
Expand Down Expand Up @@ -1599,10 +1608,13 @@ TEST(CropOrPad7D, 7DView) {
for (int i2 = 0; i2 < n2; i2++) {
for (int i1 = 0; i1 < n1; i1++) {
for (int i0 = 0; i0 < n0; i0++) {
if (i0 >= h_ref_x.extent(0) || i1 >= h_ref_x.extent(1) ||
i2 >= h_ref_x.extent(2) || i3 >= h_ref_x.extent(3) ||
i4 >= h_ref_x.extent(4) || i5 >= h_ref_x.extent(5) ||
i6 >= h_ref_x.extent(6))
if (static_cast<std::size_t>(i0) >= h_ref_x.extent(0) ||
static_cast<std::size_t>(i1) >= h_ref_x.extent(1) ||
static_cast<std::size_t>(i2) >= h_ref_x.extent(2) ||
static_cast<std::size_t>(i3) >= h_ref_x.extent(3) ||
static_cast<std::size_t>(i4) >= h_ref_x.extent(4) ||
static_cast<std::size_t>(i5) >= h_ref_x.extent(5) ||
static_cast<std::size_t>(i6) >= h_ref_x.extent(6))
continue;
h_ref_x(i0, i1, i2, i3, i4, i5, i6) =
h_x(i0, i1, i2, i3, i4, i5, i6);
Expand Down Expand Up @@ -1666,13 +1678,14 @@ TEST(CropOrPad8D, 8DView) {
for (int i2 = 0; i2 < n2; i2++) {
for (int i1 = 0; i1 < n1; i1++) {
for (int i0 = 0; i0 < n0; i0++) {
if (i0 >= h_ref_x.extent(0) ||
i1 >= h_ref_x.extent(1) ||
i2 >= h_ref_x.extent(2) ||
i3 >= h_ref_x.extent(3) ||
i4 >= h_ref_x.extent(4) ||
i5 >= h_ref_x.extent(5) ||
i6 >= h_ref_x.extent(6) || i7 >= h_ref_x.extent(7))
if (static_cast<std::size_t>(i0) >= h_ref_x.extent(0) ||
static_cast<std::size_t>(i1) >= h_ref_x.extent(1) ||
static_cast<std::size_t>(i2) >= h_ref_x.extent(2) ||
static_cast<std::size_t>(i3) >= h_ref_x.extent(3) ||
static_cast<std::size_t>(i4) >= h_ref_x.extent(4) ||
static_cast<std::size_t>(i5) >= h_ref_x.extent(5) ||
static_cast<std::size_t>(i6) >= h_ref_x.extent(6) ||
static_cast<std::size_t>(i7) >= h_ref_x.extent(7))
continue;
h_ref_x(i0, i1, i2, i3, i4, i5, i6, i7) =
h_x(i0, i1, i2, i3, i4, i5, i6, i7);
Expand Down
Loading
Loading