Change the value of BlockRandomAccessSparseMatrix::kMaxRowBlocks
1. Rename it to kRowShift.
2. Make it a static constexpr.
3. Change it to 2^32, which should allow for easier bit arithmetic
for the compiler than the previously used value of 10000000.
4. Change the name of the two associated private methods from
IntPairToLong to IntPairToInt64 and LongToIntPair to Int64ToIntPair.
Change-Id: I54d61bcf1121079b222ef518324de5cffc1be064
diff --git a/internal/ceres/block_random_access_sparse_matrix.cc b/internal/ceres/block_random_access_sparse_matrix.cc
index 784ffeb..36a3188 100644
--- a/internal/ceres/block_random_access_sparse_matrix.cc
+++ b/internal/ceres/block_random_access_sparse_matrix.cc
@@ -49,11 +49,8 @@
const std::set<std::pair<int, int>>& block_pairs,
ContextImpl* context,
int num_threads)
- : kMaxRowBlocks(10 * 1000 * 1000),
- blocks_(blocks),
- context_(context),
- num_threads_(num_threads) {
- CHECK_LT(blocks.size(), kMaxRowBlocks);
+ : blocks_(blocks), context_(context), num_threads_(num_threads) {
+ CHECK_LE(blocks.size(), std::numeric_limits<std::int32_t>::max());
const int num_cols = NumScalarEntries(blocks);
@@ -82,7 +79,7 @@
const int row_block_size = blocks_[block_pair.first].size;
const int col_block_size = blocks_[block_pair.second].size;
cell_values_.emplace_back(block_pair, values + pos);
- layout_[IntPairToLong(block_pair.first, block_pair.second)] =
+ layout_[IntPairToInt64(block_pair.first, block_pair.second)] =
std::make_unique<CellInfo>(values + pos);
pos += row_block_size * col_block_size;
}
@@ -94,7 +91,7 @@
const int row_block_size = blocks_[row_block_id].size;
const int col_block_size = blocks_[col_block_id].size;
int pos =
- layout_[IntPairToLong(row_block_id, col_block_id)]->values - values;
+ layout_[IntPairToInt64(row_block_id, col_block_id)]->values - values;
for (int r = 0; r < row_block_size; ++r) {
for (int c = 0; c < col_block_size; ++c, ++pos) {
rows[pos] = blocks_[row_block_id].position + r;
@@ -113,7 +110,7 @@
int* col,
int* row_stride,
int* col_stride) {
- const auto it = layout_.find(IntPairToLong(row_block_id, col_block_id));
+ const auto it = layout_.find(IntPairToInt64(row_block_id, col_block_id));
if (it == layout_.end()) {
return nullptr;
}
diff --git a/internal/ceres/block_random_access_sparse_matrix.h b/internal/ceres/block_random_access_sparse_matrix.h
index 8aa13c6..a64a139 100644
--- a/internal/ceres/block_random_access_sparse_matrix.h
+++ b/internal/ceres/block_random_access_sparse_matrix.h
@@ -96,16 +96,16 @@
TripletSparseMatrix* mutable_matrix() { return tsm_.get(); }
private:
- int64_t IntPairToLong(int row, int col) const {
- return row * kMaxRowBlocks + col;
+ int64_t IntPairToInt64(int row, int col) const {
+ return row * kRowShift + col;
}
- void LongToIntPair(int64_t index, int* row, int* col) const {
- *row = index / kMaxRowBlocks;
- *col = index % kMaxRowBlocks;
+ void Int64ToIntPair(int64_t index, int* row, int* col) const {
+ *row = index / kRowShift;
+ *col = index % kRowShift;
}
- const int64_t kMaxRowBlocks;
+ constexpr static int64_t kRowShift{1ll << 32};
// row/column block sizes.
const std::vector<Block> blocks_;
@@ -114,7 +114,7 @@
// A mapping from <row_block_id, col_block_id> to the position in
// the values array of tsm_ where the block is stored.
- using LayoutType = std::unordered_map<long, std::unique_ptr<CellInfo>>;
+ using LayoutType = std::unordered_map<int64_t, std::unique_ptr<CellInfo>>;
LayoutType layout_;
// In order traversal of contents of the matrix. This allows us to
diff --git a/internal/ceres/block_random_access_sparse_matrix_test.cc b/internal/ceres/block_random_access_sparse_matrix_test.cc
index 0e21a1e..2e7d439 100644
--- a/internal/ceres/block_random_access_sparse_matrix_test.cc
+++ b/internal/ceres/block_random_access_sparse_matrix_test.cc
@@ -131,7 +131,7 @@
<< "expected: " << expected_y.transpose() << "matrix: \n " << dense;
}
-// IntPairToLong is private, thus this fixture is needed to access and
+// IntPairToInt64 is private, thus this fixture is needed to access and
// test it.
class BlockRandomAccessSparseMatrixTest : public ::testing::Test {
public:
@@ -144,21 +144,21 @@
blocks, block_pairs, &context_, 1);
}
- void CheckIntPairToLong(int a, int b) {
- int64_t value = m_->IntPairToLong(a, b);
+ void CheckIntPairToInt64(int a, int b) {
+ int64_t value = m_->IntPairToInt64(a, b);
EXPECT_GT(value, 0) << "Overflow a = " << a << " b = " << b;
EXPECT_GT(value, a) << "Overflow a = " << a << " b = " << b;
EXPECT_GT(value, b) << "Overflow a = " << a << " b = " << b;
}
- void CheckLongToIntPair() {
- uint64_t max_rows = m_->kMaxRowBlocks;
+ void CheckInt64ToIntPair() {
+ uint64_t max_rows = m_->kRowShift;
for (int row = max_rows - 10; row < max_rows; ++row) {
for (int col = 0; col < 10; ++col) {
int row_computed;
int col_computed;
- m_->LongToIntPair(
- m_->IntPairToLong(row, col), &row_computed, &col_computed);
+ m_->Int64ToIntPair(
+ m_->IntPairToInt64(row, col), &row_computed, &col_computed);
EXPECT_EQ(row, row_computed);
EXPECT_EQ(col, col_computed);
}
@@ -170,13 +170,13 @@
std::unique_ptr<BlockRandomAccessSparseMatrix> m_;
};
-TEST_F(BlockRandomAccessSparseMatrixTest, IntPairToLongOverflow) {
- CheckIntPairToLong(std::numeric_limits<int>::max(),
- std::numeric_limits<int>::max());
+TEST_F(BlockRandomAccessSparseMatrixTest, IntPairToInt64Overflow) {
+ CheckIntPairToInt64(std::numeric_limits<int32_t>::max(),
+ std::numeric_limits<int32_t>::max());
}
-TEST_F(BlockRandomAccessSparseMatrixTest, LongToIntPair) {
- CheckLongToIntPair();
+TEST_F(BlockRandomAccessSparseMatrixTest, Int64ToIntPair) {
+ CheckInt64ToIntPair();
}
} // namespace ceres::internal
