Skip to content

Commit

Permalink
spgemm: remove checks for identical A/B graph pointers
Browse files Browse the repository at this point in the history
  • Loading branch information
brian-kelley committed Mar 17, 2023
1 parent 1561b65 commit 87fc970
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 69 deletions.
56 changes: 12 additions & 44 deletions sparse/src/KokkosSparse_spgemm_handle.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -779,18 +779,12 @@ class SPGEMMHandle {

private:
// An SpGEMM handle can be reused for multiple products C = A*B, but only if
// the sparsity patterns of A and B do not change. Enforce this by recording
// the raw data pointers of the matrices' rowptrs and entries during symbolic
// and numeric, and make sure they never change.
const void *a_rowptrs = nullptr, *a_entries = nullptr;
const void *b_rowptrs = nullptr, *b_entries = nullptr;
const void *c_rowptrs = nullptr, *c_entries = nullptr;
// In a debug build, also hash A,B rowptrs/entries to make sure their actual
// contents do not change.
#ifndef NDEBUG
uint32_t a_graph_hash = 0U;
uint32_t b_graph_hash = 0U;
#endif
// the sparsity patterns of A and B do not change. Enforce this (in debug
// builds only) by recording hashes of the graphs, and then checking they
// match in later calls.
bool computedInputHashes = false;
uint32_t a_graph_hash = 0U;
uint32_t b_graph_hash = 0U;

public:
template <typename a_rowptrs_t, typename a_entries_t, typename b_rowptrs_t,
Expand All @@ -800,59 +794,33 @@ class SPGEMMHandle {
const b_rowptrs_t &b_rowptrsIn,
const b_entries_t &b_entriesIn,
const c_rowptrs_t &c_rowptrsIn) {
#ifndef NDEBUG
// If this is the first symbolic call, assign the handle's CRS pointers to
// check against later
if (!a_rowptrs) {
a_rowptrs = a_rowptrsIn.data();
b_rowptrs = b_rowptrsIn.data();
a_entries = a_entriesIn.data();
b_entries = b_entriesIn.data();
c_rowptrs = c_rowptrsIn.data();
#ifndef NDEBUG
if (!computedInputHashes) {
a_graph_hash = KokkosKernels::Impl::hashView(a_rowptrsIn) ^
KokkosKernels::Impl::hashView(a_entriesIn);
b_graph_hash = KokkosKernels::Impl::hashView(b_rowptrsIn) ^
KokkosKernels::Impl::hashView(b_entriesIn);
#endif
computedInputHashes = true;
} else {
// Not the first call: make sure all views match what was passed to the
// first call
if (a_rowptrs != a_rowptrsIn.data() || a_entries != a_entriesIn.data())
return false;
if (b_rowptrs != b_rowptrsIn.data() || b_entries != b_entriesIn.data())
return false;
if (c_rowptrs != c_rowptrsIn.data()) return false;
#ifndef NDEBUG
if (a_graph_hash != (KokkosKernels::Impl::hashView(a_rowptrsIn) ^
KokkosKernels::Impl::hashView(a_entriesIn)))
return false;
if (b_graph_hash != (KokkosKernels::Impl::hashView(b_rowptrsIn) ^
KokkosKernels::Impl::hashView(b_entriesIn)))
return false;
#endif
}
#endif
return true;
}

template <typename a_rowptrs_t, typename a_entries_t, typename b_rowptrs_t,
typename b_entries_t, typename c_rowptrs_t, typename c_entries_t>
typename b_entries_t>
bool checkMatrixIdentitiesNumeric(const a_rowptrs_t &a_rowptrsIn,
const a_entries_t &a_entriesIn,
const b_rowptrs_t &b_rowptrsIn,
const b_entries_t &b_entriesIn,
const c_rowptrs_t &c_rowptrsIn,
const c_entries_t &c_entriesIn) {
// A, B rowptrs and entries (pointers and hashes) will have already been
// set. If this is the first numeric call, assign the pointer c_entries
if (!c_entries) {
c_entries = c_entriesIn.data();
}
if (a_rowptrs != a_rowptrsIn.data() || a_entries != a_entriesIn.data())
return false;
if (b_rowptrs != b_rowptrsIn.data() || b_entries != b_entriesIn.data())
return false;
if (c_rowptrs != c_rowptrsIn.data() || c_entries != c_entriesIn.data())
return false;
const b_entries_t &b_entriesIn) {
#ifndef NDEBUG
if (a_graph_hash != (KokkosKernels::Impl::hashView(a_rowptrsIn) ^
KokkosKernels::Impl::hashView(a_entriesIn)))
Expand Down
5 changes: 2 additions & 3 deletions sparse/src/KokkosSparse_spgemm_numeric.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -245,12 +245,11 @@ void spgemm_numeric(KernelHandle *handle,
}

if (!spgemmHandle->checkMatrixIdentitiesNumeric(const_a_r, const_a_l,
const_b_r, const_b_l,
const_c_r, nonconst_c_l)) {
const_b_r, const_b_l)) {
throw std::invalid_argument(
"KokkosSparse::spgemm_numeric: once used, an spgemm handle cannot be "
"reused for a product with a different sparsity pattern.\n"
"The rowptrs and entries of A, B and C must be identical to those "
"The rowptrs and entries of A and B must be identical to those "
"passed to the first spgemm_symbolic and spgemm_numeric calls.");
}

Expand Down
2 changes: 1 addition & 1 deletion sparse/src/KokkosSparse_spgemm_symbolic.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ void spgemm_symbolic(KernelHandle *handle,
throw std::invalid_argument(
"KokkosSparse::spgemm_symbolic: once used, an spgemm handle cannot be "
"reused for a product with a different sparsity pattern.\n"
"The rowptrs and entries of A, B and C must be identical to those "
"The rowptrs and entries of A and B must be identical to those "
"passed to the first spgemm_symbolic call.");
}

Expand Down
28 changes: 7 additions & 21 deletions sparse/unit_test/Test_Sparse_spgemm.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -487,44 +487,30 @@ template <typename scalar_t, typename lno_t, typename size_type,
typename device>
void test_issue1738() {
// Make sure that std::invalid_argument is thrown if you:
// - try to reuse an SPGEMM handle on a different product
// - call symbolic and then numeric with different matrices
// - (in a debug build) call numeric where the contents (but not pointers) of
// an input matrix's entries have changed
// - call numeric where an input matrix's entries have changed.
// - try to reuse an spgemm handle by calling symbolic with new input
// matrices
// This check is only enabled in debug builds.
#ifndef NDEBUG
using crsMat_t = CrsMatrix<scalar_t, lno_t, device, void, size_type>;
using KernelHandle = KokkosKernels::Experimental::KokkosKernelsHandle<
size_type, lno_t, scalar_t, typename device::execution_space,
typename device::memory_space, typename device::memory_space>;
crsMat_t A1 = KokkosSparse::Impl::kk_generate_diag_matrix<crsMat_t>(100);
crsMat_t B1 = KokkosSparse::Impl::kk_generate_diag_matrix<crsMat_t>(100);
crsMat_t A2 = KokkosSparse::Impl::kk_generate_diag_matrix<crsMat_t>(100);
crsMat_t B2 = KokkosSparse::Impl::kk_generate_diag_matrix<crsMat_t>(100);
crsMat_t A2 = KokkosSparse::Impl::kk_generate_diag_matrix<crsMat_t>(50);
crsMat_t B2 = KokkosSparse::Impl::kk_generate_diag_matrix<crsMat_t>(50);
{
// Test 1: reusing handle on a completely different product
KernelHandle kh;
// First, multiply A1*B1 using an SpGEMM handle
kh.create_spgemm_handle();
crsMat_t C1;
KokkosSparse::spgemm_symbolic(kh, A1, false, B1, false, C1);
KokkosSparse::spgemm_numeric(kh, A1, false, B1, false, C1);
// Then try to do A2*B2
crsMat_t C2;
EXPECT_THROW(KokkosSparse::spgemm_symbolic(kh, A2, false, B2, false, C2),
std::invalid_argument);
}
{
// Test 2: passing different B matrices to symbolic and numeric
KernelHandle kh;
kh.create_spgemm_handle();
crsMat_t C1;
KokkosSparse::spgemm_symbolic(kh, A1, false, B1, false, C1);
EXPECT_THROW(KokkosSparse::spgemm_numeric(kh, A1, false, B2, false, C1),
std::invalid_argument);
}
#ifndef NDEBUG
{
// Test 3: contents of input matrix's entries have changed.
// This check is only enabled in a debug build.
KernelHandle kh;
kh.create_spgemm_handle();
crsMat_t C1;
Expand Down

0 comments on commit 87fc970

Please sign in to comment.