Fix a bug in InnerProductComputer.

InnerProductComputer was assuming that the input matrix had
at least one structurally non-zero block. As a result sometimes
when InnerProductComputer.SubMatrix test generated matrices
where the submatrix was structurally zero it would cause
a segmentation fault.

This changes fixes this issue and reduces the threshold for the
minimum block density so that there is a much higher chance of
such matrices occuring as part of testing.

Fixes https://github.com/ceres-solver/ceres-solver/issues/820

Change-Id: Iec3a2431e646f0c2aac9e0b994531fa72323f329
This commit is contained in:
Sameer Agarwal
2022-08-07 12:54:15 -07:00
parent d8dad14eed
commit 37516c968c
2 changed files with 39 additions and 37 deletions
+8 -1
View File
@@ -77,6 +77,10 @@ int InnerProductComputer::ComputeNonzeros(
row_nnz->resize(blocks.size());
std::fill(row_nnz->begin(), row_nnz->end(), 0);
if (product_terms.empty()) {
return 0;
}
// First product term.
(*row_nnz)[product_terms[0].row] = blocks[product_terms[0].col].size;
int num_nonzeros =
@@ -197,6 +201,10 @@ void InnerProductComputer::ComputeOffsetsAndCreateResultMatrix(
*(crsm_rows + 1) = *crsm_rows + row_block_nnz[i];
}
}
result_offsets_.resize(product_terms.size());
if (num_nonzeros == 0) {
return;
}
// The following macro FILL_CRSM_COL_BLOCK is key to understanding
// how this class works.
@@ -243,7 +251,6 @@ void InnerProductComputer::ComputeOffsetsAndCreateResultMatrix(
} \
}
result_offsets_.resize(product_terms.size());
int col_nnz = 0;
int nnz = 0;
+31 -36
View File
@@ -44,36 +44,36 @@
namespace ceres {
namespace internal {
#define COMPUTE_AND_COMPARE \
{ \
inner_product_computer->Compute(); \
CompressedRowSparseMatrix* actual_product_crsm = \
inner_product_computer->mutable_result(); \
Matrix actual_inner_product = \
Eigen::Map<Eigen::SparseMatrix<double, Eigen::ColMajor>>( \
actual_product_crsm->num_rows(), \
actual_product_crsm->num_rows(), \
actual_product_crsm->num_nonzeros(), \
actual_product_crsm->mutable_rows(), \
actual_product_crsm->mutable_cols(), \
actual_product_crsm->mutable_values()); \
EXPECT_EQ(actual_inner_product.rows(), actual_inner_product.cols()); \
EXPECT_EQ(expected_inner_product.rows(), expected_inner_product.cols()); \
EXPECT_EQ(actual_inner_product.rows(), expected_inner_product.rows()); \
Matrix expected_t, actual_t; \
if (actual_product_crsm->storage_type() == \
CompressedRowSparseMatrix::StorageType::LOWER_TRIANGULAR) { \
expected_t = expected_inner_product.triangularView<Eigen::Upper>(); \
actual_t = actual_inner_product.triangularView<Eigen::Upper>(); \
} else { \
expected_t = expected_inner_product.triangularView<Eigen::Lower>(); \
actual_t = actual_inner_product.triangularView<Eigen::Lower>(); \
} \
EXPECT_LE((expected_t - actual_t).norm() / actual_t.norm(), \
100 * std::numeric_limits<double>::epsilon()) \
<< "expected: \n" \
<< expected_t << "\nactual: \n" \
<< actual_t; \
#define COMPUTE_AND_COMPARE \
{ \
inner_product_computer->Compute(); \
CompressedRowSparseMatrix* actual_product_crsm = \
inner_product_computer->mutable_result(); \
Matrix actual_inner_product = \
Eigen::Map<Eigen::SparseMatrix<double, Eigen::ColMajor>>( \
actual_product_crsm->num_rows(), \
actual_product_crsm->num_rows(), \
actual_product_crsm->num_nonzeros(), \
actual_product_crsm->mutable_rows(), \
actual_product_crsm->mutable_cols(), \
actual_product_crsm->mutable_values()); \
EXPECT_EQ(actual_inner_product.rows(), actual_inner_product.cols()); \
EXPECT_EQ(expected_inner_product.rows(), expected_inner_product.cols()); \
EXPECT_EQ(actual_inner_product.rows(), expected_inner_product.rows()); \
Matrix expected_t, actual_t; \
if (actual_product_crsm->storage_type() == \
CompressedRowSparseMatrix::StorageType::LOWER_TRIANGULAR) { \
expected_t = expected_inner_product.triangularView<Eigen::Upper>(); \
actual_t = actual_inner_product.triangularView<Eigen::Upper>(); \
} else { \
expected_t = expected_inner_product.triangularView<Eigen::Lower>(); \
actual_t = actual_inner_product.triangularView<Eigen::Lower>(); \
} \
EXPECT_LE((expected_t - actual_t).norm(), \
100 * std::numeric_limits<double>::epsilon() * actual_t.norm()) \
<< "expected: \n" \
<< expected_t << "\nactual: \n" \
<< actual_t; \
}
TEST(InnerProductComputer, NormalOperation) {
@@ -99,7 +99,6 @@ TEST(InnerProductComputer, NormalOperation) {
options.min_col_block_size = 1;
options.max_col_block_size = 10;
options.block_density = std::max(0.1, RandDouble());
VLOG(2) << "num row blocks: " << options.num_row_blocks;
VLOG(2) << "num col blocks: " << options.num_col_blocks;
VLOG(2) << "min row block size: " << options.min_row_block_size;
@@ -141,10 +140,6 @@ TEST(InnerProductComputer, NormalOperation) {
}
TEST(InnerProductComputer, SubMatrix) {
// "Randomly generated seed."
// FIXME Use a seed different from the previous one to allow the test to pass
// without a segmentation fault when compiled using 32 bit MinGW.
SetRandomState(29824);
const int kNumRowBlocks = 10;
const int kNumColBlocks = 20;
const int kNumTrials = 5;
@@ -160,7 +155,7 @@ TEST(InnerProductComputer, SubMatrix) {
options.max_row_block_size = 5;
options.min_col_block_size = 1;
options.max_col_block_size = 10;
options.block_density = std::max(0.1, RandDouble());
options.block_density = std::min(0.01, RandDouble());
VLOG(2) << "num row blocks: " << options.num_row_blocks;
VLOG(2) << "num col blocks: " << options.num_col_blocks;