Skip to content

Commit

Permalink
Turned on all compiler warnings.
Browse files Browse the repository at this point in the history 
Fixed all the associated issues in tests and library code.
  • Loading branch information
@LTLA
LTLA committed Jun 22, 2024
1 parent 9002a5e commit 0fec4df
Show file tree
Hide file tree
Showing 9 changed files with 40 additions and 36 deletions.
4 changes: 2 additions & 2 deletions include/irlba/MockMatrix.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ class MockMatrix {
* depending on what is most convenient for defining the associated `Workspace`.
*/
template<class Right_, class EigenVector_>
void multiply(const Right_& rhs, Workspace& work, EigenVector_& out) const {
void multiply(const Right_& rhs, [[maybe_unused]] Workspace& work, EigenVector_& out) const {
out.noalias() = my_x * rhs;
}

Expand All @@ -112,7 +112,7 @@ class MockMatrix {
* depending on what is most convenient for defining the associated `AdjointWorkspace`.
*/
template<class Right_, class EigenVector_>
void adjoint_multiply(const Right_& rhs, AdjointWorkspace& work, EigenVector_& out) const {
void adjoint_multiply(const Right_& rhs, [[maybe_unused]] AdjointWorkspace& work, EigenVector_& out) const {
out.noalias() = my_x.adjoint() * rhs;
}

Expand Down
28 changes: 14 additions & 14 deletions include/irlba/parallel.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ class ParallelSparseMatrix {
my_primary_starts.resize(my_nthreads);
my_primary_ends.resize(my_nthreads);
{
size_t primary_counter = 0;
Eigen::Index primary_counter = 0;
PointerType sofar = per_thread;
for (int t = 0; t < my_nthreads; ++t) {
my_primary_starts[t] = primary_counter;
Expand Down Expand Up @@ -224,7 +224,7 @@ class ParallelSparseMatrix {
sofar += per_thread;
}

for (size_t c = 0; c < my_primary_dim; ++c) {
for (Eigen::Index c = 0; c < my_primary_dim; ++c) {
auto primary_start = my_ptrs[c], primary_end = my_ptrs[c + 1];
my_secondary_nonzero_starts[0][c] = primary_start;

Expand All @@ -245,7 +245,7 @@ class ParallelSparseMatrix {
output.setZero();

if (my_nthreads == 1) {
for (size_t c = 0; c < my_primary_dim; ++c) {
for (Eigen::Index c = 0; c < my_primary_dim; ++c) {
auto start = my_ptrs[c];
auto end = my_ptrs[c + 1];
auto val = rhs.coeff(c);
Expand All @@ -267,7 +267,7 @@ class ParallelSparseMatrix {

const auto& starts = my_secondary_nonzero_starts[t];
const auto& ends = my_secondary_nonzero_starts[t + 1];
for (size_t c = 0; c < my_primary_dim; ++c) {
for (Eigen::Index c = 0; c < my_primary_dim; ++c) {
auto start = starts[c];
auto end = ends[c];
auto val = rhs.coeff(c);
Expand All @@ -288,7 +288,7 @@ class ParallelSparseMatrix {
template<class Right_, class EigenVector_>
void direct_multiply(const Right_& rhs, EigenVector_& output) const {
if (my_nthreads == 1) {
for (size_t c = 0; c < my_primary_dim; ++c) {
for (Eigen::Index c = 0; c < my_primary_dim; ++c) {
output.coeffRef(c) = column_dot_product<typename EigenVector_::Scalar>(c, rhs);
}
return;
Expand Down Expand Up @@ -347,7 +347,7 @@ class ParallelSparseMatrix {

public:
template<class Right_, class EigenVector_>
void multiply(const Right_& rhs, Workspace& work, EigenVector_& output) const {
void multiply(const Right_& rhs, [[maybe_unused]] Workspace& work, EigenVector_& output) const {
if (my_column_major) {
indirect_multiply(rhs, output);
} else {
Expand All @@ -356,7 +356,7 @@ class ParallelSparseMatrix {
}

template<class Right_, class EigenVector_>
void adjoint_multiply(const Right_& rhs, AdjointWorkspace& work, EigenVector_& output) const {
void adjoint_multiply(const Right_& rhs, [[maybe_unused]] AdjointWorkspace& work, EigenVector_& output) const {
if (my_column_major) {
direct_multiply(rhs, output);
} else {
Expand All @@ -372,16 +372,16 @@ class ParallelSparseMatrix {
output.setZero();

if (my_column_major) {
for (size_t c = 0; c < nc; ++c) {
size_t col_start = my_ptrs[c], col_end = my_ptrs[c + 1];
for (size_t s = col_start; s < col_end; ++s) {
for (Eigen::Index c = 0; c < nc; ++c) {
PointerType col_start = my_ptrs[c], col_end = my_ptrs[c + 1];
for (PointerType s = col_start; s < col_end; ++s) {
output.coeffRef(my_indices[s], c) = my_values[s];
}
}
} else {
for (size_t r = 0; r < nr; ++r) {
size_t row_start = my_ptrs[r], row_end = my_ptrs[r + 1];
for (size_t s = row_start; s < row_end; ++s) {
for (Eigen::Index r = 0; r < nr; ++r) {
PointerType row_start = my_ptrs[r], row_end = my_ptrs[r + 1];
for (PointerType s = row_start; s < row_end; ++s) {
output.coeffRef(r, my_indices[s]) = my_values[s];
}
}
Expand Down Expand Up @@ -411,7 +411,7 @@ class EigenThreadScope {
/**
* @param n Number of threads to be used by Eigen.
*/
EigenThreadScope(int n)
EigenThreadScope([[maybe_unused]] int n)
#ifdef _OPENMP
: my_previous(Eigen::nbThreads()) {
#ifndef IRLBA_CUSTOM_PARALLEL
Expand Down
4 changes: 4 additions & 0 deletions tests/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ target_link_libraries(
irlba
)

target_compile_options(libtest PRIVATE -Wall -Werror -Wpedantic -Wextra)

set(CODE_COVERAGE "Enable coverage testing" OFF)
set(DO_CODE_COVERAGE OFF)
if(CODE_COVERAGE AND CMAKE_CXX_COMPILER_ID MATCHES "GNU|Clang")
Expand All @@ -50,6 +52,7 @@ add_executable(
)

target_compile_definitions(custom_parallel PRIVATE TEST_CUSTOM_PARALLEL)
target_compile_options(custom_parallel PRIVATE -Wall -Werror -Wpedantic -Wextra)

target_link_libraries(
custom_parallel
Expand Down Expand Up @@ -78,6 +81,7 @@ if (OpenMP_FOUND)
irlba
)

target_compile_options(omptest PRIVATE -Wall -Werror -Wpedantic -Wextra)
target_link_libraries(omptest OpenMP::OpenMP_CXX)
gtest_discover_tests(omptest)

Expand Down
10 changes: 5 additions & 5 deletions tests/src/compare.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ void expect_equal_column_vectors(const Eigen::MatrixXd& left, const Eigen::Matri
ASSERT_EQ(left.cols(), right.cols());
ASSERT_EQ(left.rows(), right.rows());

for (size_t i = 0; i < left.cols(); ++i) {
for (size_t j = 0; j < left.rows(); ++j) {
for (Eigen::Index i = 0; i < left.cols(); ++i) {
for (Eigen::Index j = 0; j < left.rows(); ++j) {
EXPECT_TRUE(same_same(std::abs(left(j, i)), std::abs(right(j, i)), tol));
}

Expand All @@ -33,16 +33,16 @@ void expect_equal_column_vectors(const Eigen::MatrixXd& left, const Eigen::Matri
inline void expect_equal_matrix(const Eigen::MatrixXd& left, const Eigen::MatrixXd& right, double tol=1e-8) {
ASSERT_EQ(left.cols(), right.cols());
ASSERT_EQ(left.rows(), right.rows());
for (size_t i = 0; i < left.cols(); ++i) {
for (size_t j = 0; j < left.rows(); ++j) {
for (Eigen::Index i = 0; i < left.cols(); ++i) {
for (Eigen::Index j = 0; j < left.rows(); ++j) {
EXPECT_TRUE(same_same(left(j, i), right(j, i), tol));
}
}
}

inline void expect_equal_vectors(const Eigen::VectorXd& left, const Eigen::VectorXd& right, double tol=1e-8) {
ASSERT_EQ(left.size(), right.size());
for (size_t i = 0; i < left.size(); ++i) {
for (Eigen::Index i = 0; i < left.size(); ++i) {
EXPECT_TRUE(same_same(left[i], right[i], tol));
}
return;
Expand Down
6 changes: 3 additions & 3 deletions tests/src/invariant.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ TEST_P(InvariantTester, SingularCheck) {
// Comparing the sum of squared singular values to the Frobenius norm;
// they should be the same.
double total_var = 0;
for (size_t r = 0; r < res.D.size(); ++r) {
for (Eigen::Index r = 0; r < res.D.size(); ++r) {
total_var += res.D[r] * res.D[r];
}

Expand All @@ -71,8 +71,8 @@ TEST_P(InvariantTester, Recovery) {
ASSERT_EQ(recovered.rows(), A.rows());
ASSERT_EQ(recovered.cols(), A.cols());

for (size_t c = 0; c < A.cols(); ++c) {
for (size_t r = 0; r < A.rows(); ++r) {
for (Eigen::Index c = 0; c < A.cols(); ++c) {
for (Eigen::Index r = 0; r < A.rows(); ++r) {
same_same(recovered(r, c), A(r, c), 1e-8);
}
}
Expand Down
8 changes: 4 additions & 4 deletions tests/src/irlba.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ TEST_P(IrlbaTester, CenterScale) {
expect_equal_vectors(res.D, res2.D);
expect_equal_column_vectors<true>(res.U, res2.U);
expect_equal_column_vectors(res.V, res2.V);
for (size_t i = 0; i < res2.U.cols(); ++i) {
for (Eigen::Index i = 0; i < res2.U.cols(); ++i) {
EXPECT_TRUE(std::abs(res2.U.col(i).sum()) < 1e-8);
}
}
Expand All @@ -113,7 +113,7 @@ TEST_P(IrlbaTester, CenterScale) {
expect_equal_vectors(res.D, res2.D);
expect_equal_column_vectors<true>(res.U, res2.U);
expect_equal_column_vectors(res.V, res2.V);
for (size_t i = 0; i < res2.U.cols(); ++i) {
for (Eigen::Index i = 0; i < res2.U.cols(); ++i) {
EXPECT_TRUE(std::abs(res2.U.col(i).sum()) < 1e-8);
}
EXPECT_NE(ref.D, res2.D);
Expand Down Expand Up @@ -193,7 +193,7 @@ TEST(IrlbaTest, SmallExactCenterScale) {
expect_equal_vectors(res.D, res2.D);
expect_equal_column_vectors(res.U, res2.U);
expect_equal_column_vectors(res.V, res2.V);
for (size_t i = 0; i < res2.U.cols(); ++i) {
for (Eigen::Index i = 0; i < res2.U.cols(); ++i) {
EXPECT_TRUE(std::abs(res2.U.col(i).sum()) < 1e-8);
}
}
Expand All @@ -204,7 +204,7 @@ TEST(IrlbaTest, SmallExactCenterScale) {
expect_equal_vectors(res.D, res2.D);
expect_equal_column_vectors(res.U, res2.U);
expect_equal_column_vectors(res.V, res2.V);
for (size_t i = 0; i < res2.U.cols(); ++i) {
for (Eigen::Index i = 0; i < res2.U.cols(); ++i) {
EXPECT_TRUE(std::abs(res2.U.col(i).sum()) < 1e-8);
}
EXPECT_NE(ref.D, res2.D);
Expand Down
8 changes: 4 additions & 4 deletions tests/src/lanczos.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,14 +143,14 @@ TEST_P(LanczosTester, Restart) {
irlba::internal::LanczosWorkspace<Eigen::VectorXd, Eigen::MatrixXd> init2(A);
irlba::internal::run_lanczos_bidiagonalization(A, W, V, B, eng, init2, 0, opt);

for (size_t i = 0; i < copyW.cols(); ++i) {
for (size_t j = 0; j < copyW.rows(); ++j) {
for (Eigen::Index i = 0; i < copyW.cols(); ++i) {
for (Eigen::Index j = 0; j < copyW.rows(); ++j) {
EXPECT_FLOAT_EQ(copyW(j, i), W(j, i));
}
}

for (size_t i = 0; i < copyV.cols(); ++i) {
for (size_t j = 0; j < copyV.rows(); ++j) {
for (Eigen::Index i = 0; i < copyV.cols(); ++i) {
for (Eigen::Index j = 0; j < copyV.rows(); ++j) {
EXPECT_FLOAT_EQ(copyV(j, i), V(j, i));
}
}
Expand Down
4 changes: 2 additions & 2 deletions tests/src/sparse.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,8 @@ TEST_F(SparseTester, CenterScale) {
expect_equal_column_vectors(res.V, res2.V);

// Don't compare U, as this will always be zero.
for (size_t i = 0; i < res.U.cols(); ++i) {
for (size_t j = 0; j < res.U.rows(); ++j) {
for (Eigen::Index i = 0; i < res.U.cols(); ++i) {
for (Eigen::Index j = 0; j < res.U.rows(); ++j) {
double labs = std::abs(res.U(j, i));
double rabs = std::abs(res2.U(j, i));
EXPECT_TRUE(same_same(labs, rabs, 1e-8));
Expand Down
4 changes: 2 additions & 2 deletions tests/src/utils.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ TEST(UtilsTest, FillNormals) {
}

std::sort(test.begin(), test.end());
for (size_t v = 1; v < test.size(); ++v) {
for (Eigen::Index v = 1; v < test.size(); ++v) {
EXPECT_NE(test[v], test[v-1]);
}

Expand All @@ -44,7 +44,7 @@ TEST(UtilsTest, FillNormals) {
}

std::sort(test2.begin(), test2.end());
for (size_t v = 1; v < test2.size(); ++v) {
for (Eigen::Index v = 1; v < test2.size(); ++v) {
EXPECT_NE(test2[v], test2[v-1]);
}

Expand Down

0 comments on commit 0fec4df

Please sign in to comment.