Skip to content

Commit

Permalink
Fix a bug in copying an empty matrix into a bigger matrix (#666)
Browse files Browse the repository at this point in the history
* add unit test; do not copy empty matrix

* delete variables

* fix typo
  • Loading branch information
nychiang authored Oct 13, 2023
1 parent 2919a87 commit c6f94d2
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 6 deletions.
4 changes: 4 additions & 0 deletions src/LinAlg/hiopMatrixRajaSparseTripletImpl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -1114,6 +1114,10 @@ copyRowsBlockFrom(const hiopMatrix& src_gen,
const index_type& rows_dst_idx_st,
const size_type& dest_nnz_st)
{
if(n_rows <= 0) {
return;
}

const hiopMatrixRajaSparseTriplet& src = dynamic_cast<const hiopMatrixRajaSparseTriplet&>(src_gen);
assert(this->numberOfNonzeros() >= src.numberOfNonzeros());
assert(this->n() >= src.n());
Expand Down
2 changes: 1 addition & 1 deletion tests/LinAlg/matrixTestsRajaSparseTriplet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,7 @@ void MatrixTestsRajaSparseTriplet::initializeMatrix(

assert(A->numberOfNonzeros() == m * entries_per_row && "Matrix initialized with insufficent number of non-zero entries");
A->copyFromDev();
for(local_ordinal_type row = 0, col = 0, i = 0; row < m; row++, col = 0)
for(local_ordinal_type row = 0, col = 0, i = 0; row < m && i < A->numberOfNonzeros(); row++, col = 0)
{
for(local_ordinal_type j=0; j<entries_per_row-1; i++, j++, col += n / entries_per_row)
{
Expand Down
2 changes: 1 addition & 1 deletion tests/LinAlg/matrixTestsSparse.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -1034,7 +1034,7 @@ class MatrixTestsSparse : public TestBase
assert(A.n() >= B.n());
assert(n_rows + A_rows_st <= A.m());
assert(n_rows + B_rows_st <= B.m());
assert(nnz_A_need_to_copy<A_nnz);
assert(nnz_A_need_to_copy<=A_nnz);
assert(nnz_A_need_to_copy+B_nnz_st<=B_nnz);

const real_type A_val = one;
Expand Down
2 changes: 1 addition & 1 deletion tests/LinAlg/matrixTestsSparseTriplet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ void MatrixTestsSparseTriplet::initializeMatrix(

assert(A->numberOfNonzeros() == m * entries_per_row && "Matrix initialized with insufficent number of non-zero entries");

for(local_ordinal_type row = 0, col = 0, i = 0; row < m; row++, col = 0)
for(local_ordinal_type row = 0, col = 0, i = 0; row < m && i < A->numberOfNonzeros(); row++, col = 0)
{
for(local_ordinal_type j=0; j<entries_per_row-1; i++, j++, col += n / entries_per_row)
{
Expand Down
32 changes: 29 additions & 3 deletions tests/testMatrixSparse.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,15 @@ int main(int argc, char** argv)
hiop::hiopMatrixSparse* mxn_sparse =
hiop::LinearAlgebraFactory::create_matrix_sparse(mem_space, M_local, N_local, nnz);
test.initializeMatrix(mxn_sparse, entries_per_row);


hiop::hiopMatrixSparse* mxn_empty =
hiop::LinearAlgebraFactory::create_matrix_sparse(mem_space, M_local, N_global, 0);
test.initializeMatrix(mxn_empty, 0);

hiop::hiopMatrixSparse* nullxn_sparse =
hiop::LinearAlgebraFactory::create_matrix_sparse(mem_space, 0, N_global, 0);
test.initializeMatrix(nullxn_sparse, 0);

hiop::hiopVectorPar vec_m(M_global);
hiop::hiopVectorPar vec_n(N_global);

Expand Down Expand Up @@ -172,7 +180,10 @@ int main(int argc, char** argv)
// copy the 1st row of mxn_sparse to the last row in m2xn_sparse
// replace the nonzero index from "nnz-entries_per_row"
fail += test.copy_rows_block_from(*mxn_sparse, *m2xn_sparse,0, 1, M_global-1, mxn_sparse->numberOfNonzeros()-entries_per_row);

fail += test.copy_rows_block_from(*mxn_empty, *m2xn_sparse,0, 1, M_global-1, mxn_sparse->numberOfNonzeros());
fail += test.copy_rows_block_from(*nullxn_sparse, *m2xn_sparse,0, 0, M_global-1, mxn_sparse->numberOfNonzeros());
fail += test.copy_rows_block_from(*nullxn_sparse, *nullxn_sparse,0, 0, 0, nullxn_sparse->numberOfNonzeros());

// create a bigger matrix, to test copy_submatrix_from and opy_submatrix_from_trans
hiop::hiopMatrixDenseRowMajor m4xn4_dense(2*M_global+N_global, 2*M_global+N_global);
local_ordinal_type nnz4 = entries_per_row*(2*M_global+N_global);
Expand Down Expand Up @@ -205,6 +216,8 @@ int main(int argc, char** argv)

// Remove testing objects
delete mxn_sparse;
delete mxn_empty;
delete nullxn_sparse;
delete m2xn_sparse;
delete m3xn3_sparse;
delete m4xn4_sparse;
Expand Down Expand Up @@ -232,7 +245,15 @@ int main(int argc, char** argv)
hiop::LinearAlgebraFactory::create_matrix_sparse(mem_space, M_local, N_local, nnz);

test.initializeMatrix(mxn_sparse, entries_per_row);

hiop::hiopMatrixSparse* mxn_empty =
hiop::LinearAlgebraFactory::create_matrix_sparse(mem_space, M_local, N_global, 0);
test.initializeMatrix(mxn_empty, 0);

hiop::hiopMatrixSparse* nullxn_sparse =
hiop::LinearAlgebraFactory::create_matrix_sparse(mem_space, 0, N_global, 0);
test.initializeMatrix(nullxn_sparse, 0);

//hiop::hiopVectorRajaPar vec_m(M_global, mem_space);
hiop::hiopVector* vec_m = hiop::LinearAlgebraFactory::create_vector(mem_space, M_global);
hiop::hiopVector* vec_m_2 = hiop::LinearAlgebraFactory::create_vector(mem_space, M_global);
Expand Down Expand Up @@ -311,7 +332,10 @@ int main(int argc, char** argv)
// copy the 1st row of mxn_sparse to the last row in m2xn_sparse
// replace the nonzero index from "nnz-entries_per_row"
fail += test.copy_rows_block_from(*mxn_sparse, *m2xn_sparse,0, 1, M_global-1, mxn_sparse->numberOfNonzeros()-entries_per_row);

fail += test.copy_rows_block_from(*mxn_empty, *m2xn_sparse,0, 1, M_global-1, mxn_sparse->numberOfNonzeros());
fail += test.copy_rows_block_from(*nullxn_sparse, *m2xn_sparse,0, 0, M_global-1, mxn_sparse->numberOfNonzeros());
fail += test.copy_rows_block_from(*nullxn_sparse, *nullxn_sparse,0, 0, 0, nullxn_sparse->numberOfNonzeros());

// create a bigger matrix, to test copy_submatrix_from and opy_submatrix_from_trans
//hiop::hiopMatrixRajaDense m4xn4_dense(2*M_global+N_global, 2*M_global+N_global,mem_space);
hiop::hiopMatrixDense* m4xn4_dense =
Expand Down Expand Up @@ -349,6 +373,8 @@ int main(int argc, char** argv)

// Remove testing objects
delete mxn_sparse;
delete mxn_empty;
delete nullxn_sparse;
delete mxn_sparse_2;
delete m2xn_sparse;
delete m3xn3_sparse;
Expand Down

0 comments on commit c6f94d2

Please sign in to comment.