Skip to content

Commit 372949c

Browse files
committed
Protect against zero-extent matrices, which cause Eigen's SVD to abort.
1 parent 56b9b0d commit 372949c

3 files changed

Lines changed: 43 additions & 17 deletions

File tree

CMakeLists.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
cmake_minimum_required(VERSION 3.14...3.25)
22

33
project(irlba
4-
VERSION 3.0.0
4+
VERSION 3.0.1
55
DESCRIPTION "C++ port of the IRLBA algorithm"
66
LANGUAGES CXX)
77

include/irlba/compute.hpp

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,15 @@ std::pair<bool, int> compute(
159159
throw std::runtime_error("requested number of singular values must be less than the smaller matrix dimension for IRLBA iterations");
160160
}
161161

162+
// Optimization if no SVs are requested. Also protect the exact SVD code
163+
// from a zero-extent matrix, as this causes Eigen to shit itself.
164+
if (requested_number == 0) {
165+
outD.resize(requested_number);
166+
outU.resize(matrix.rows(), requested_number);
167+
outV.resize(matrix.cols(), requested_number);
168+
return std::make_pair(true, 0);
169+
}
170+
162171
// Falling back to an exact SVD for small matrices or if the requested number is too large
163172
// (not enough of a workspace). Hey, I don't make the rules.
164173
if (
@@ -169,10 +178,9 @@ std::pair<bool, int> compute(
169178
return std::make_pair(true, 0);
170179
}
171180

172-
Eigen::Index work = choose_requested_plus_extra_work_or_smaller(requested_number, options.extra_work, smaller);
173-
if (work == 0) {
174-
throw std::runtime_error("number of requested dimensions must be positive");
175-
}
181+
// We know work must be positive at this point, as we would have returned early if requested_number = 0.
182+
// Similar, if smaller = 0, we would have either thrown earlier or set requested_number = 0.
183+
const Eigen::Index work = choose_requested_plus_extra_work_or_smaller(requested_number, options.extra_work, smaller);
176184

177185
// Don't worry about sanitizing dimensions for Eigen constructors,
178186
// as the former are stored as Eigen::Index and the latter accepts Eigen::Index inputs.

tests/src/compute.cpp

Lines changed: 30 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ TEST(Compute, CompareToExact) {
2222
irlba::Options opt;
2323
opt.exact_for_large_number = false;
2424
irlba::SimpleMatrix<Eigen::VectorXd, Eigen::MatrixXd, decltype(&A)> wrapped(&A);
25-
auto res = irlba::compute(wrapped, 5, opt);
25+
auto res = irlba::compute(wrapped, rank, opt);
2626

2727
Eigen::JacobiSVD<decltype(A), Eigen::ComputeThinU | Eigen::ComputeThinV> svd(A);
2828
expect_equal_vectors(res.D, svd.singularValues().head(rank), 1e-8);
@@ -49,6 +49,35 @@ TEST(Compute, ChooseRequestedPlusExtraWorkOrSmaller) {
4949
EXPECT_EQ(irlba::choose_requested_plus_extra_work_or_smaller(10, 20, 35), 30);
5050
}
5151

52+
TEST(Compute, ZeroExtent) {
53+
irlba::Options opt;
54+
opt.cap_number = true;
55+
56+
{
57+
auto A = create_random_matrix(0, 10);
58+
irlba::SimpleMatrix<Eigen::VectorXd, Eigen::MatrixXd, decltype(&A)> wrapped(&A);
59+
auto res = irlba::compute(wrapped, 5, opt);
60+
61+
EXPECT_EQ(res.D.size(), 0);
62+
EXPECT_EQ(res.U.rows(), 0);
63+
EXPECT_EQ(res.U.cols(), 0);
64+
EXPECT_EQ(res.V.rows(), 10);
65+
EXPECT_EQ(res.V.cols(), 0);
66+
}
67+
68+
{
69+
auto A = create_random_matrix(10, 0);
70+
irlba::SimpleMatrix<Eigen::VectorXd, Eigen::MatrixXd, decltype(&A)> wrapped(&A);
71+
auto res = irlba::compute(wrapped, 5, opt);
72+
73+
EXPECT_EQ(res.D.size(), 0);
74+
EXPECT_EQ(res.U.rows(), 10);
75+
EXPECT_EQ(res.U.cols(), 0);
76+
EXPECT_EQ(res.V.rows(), 0);
77+
EXPECT_EQ(res.V.cols(), 0);
78+
}
79+
}
80+
5281
TEST(Compute, UpdateK) {
5382
auto ref_update_k = [](int k, const int requested_number, const int n_converged, const int work) -> int {
5483
if (k < requested_number + n_converged) {
@@ -214,17 +243,6 @@ TEST(Compute, Fails) {
214243
}
215244
EXPECT_TRUE(message.find("must be less than") != std::string::npos);
216245

217-
// Requested number of SVs + extra work is zero.
218-
message.clear();
219-
try {
220-
irlba::Options opt;
221-
opt.extra_work = 0;
222-
irlba::compute(wrapped, 0, opt);
223-
} catch (const std::exception& e) {
224-
message = e.what();
225-
}
226-
EXPECT_TRUE(message.find("must be positive") != std::string::npos);
227-
228246
// Initialization vector is not of the right length.
229247
message.clear();
230248
try {

0 commit comments

Comments
 (0)