From 6c8ef72f2dedf92589c49ed035082500c2260428 Mon Sep 17 00:00:00 2001 From: Aristeidis Mastoras Date: Wed, 12 Oct 2022 18:56:41 +0200 Subject: [PATCH 01/14] Issue #533: clearing the output vector of set operations and fix bug in set unittest --- include/graphblas/reference/io.hpp | 15 +++++++++++++++ tests/unit/set.cpp | 4 ++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/include/graphblas/reference/io.hpp b/include/graphblas/reference/io.hpp index dbd34c199..38230d135 100644 --- a/include/graphblas/reference/io.hpp +++ b/include/graphblas/reference/io.hpp @@ -484,6 +484,11 @@ namespace grb { const auto & m_coors = internal::getCoordinates( m ); auto m_p = internal::getRaw( m ); + // make the vector empty unless the dense descriptor is provided + if( !( descr & descriptors::dense ) ) { + internal::getCoordinates( x ).clear(); + } + #ifdef _H_GRB_REFERENCE_OMP_IO #pragma omp parallel { @@ -658,6 +663,11 @@ namespace grb { OutputType * __restrict__ const dst = internal::getRaw( x ); const InputType * __restrict__ const src = internal::getRaw( y ); + // make the vector empty unless the dense descriptor is provided + if( !( descr & descriptors::dense ) ) { + internal::getCoordinates( x ).clear(); + } + // get #nonzeroes const size_t nz = internal::getCoordinates( y ).nonzeroes(); #ifdef _DEBUG @@ -813,6 +823,11 @@ namespace grb { const auto &y_coors = internal::getCoordinates( y ); auto &x_coors = internal::getCoordinates( x ); + // make the vector empty unless the dense descriptor is provided + if( !( descr & descriptors::dense ) ) { + internal::getCoordinates( x ).clear(); + } + // choose optimal loop size const bool loop_over_y = (descr & descriptors::invert_mask) || ( y_coors.nonzeroes() < m_coors.nonzeroes() ); diff --git a/tests/unit/set.cpp b/tests/unit/set.cpp index a381a4243..6f9f0919b 100644 --- a/tests/unit/set.cpp +++ b/tests/unit/set.cpp @@ -207,7 +207,7 @@ void grb_program( const size_t & n, grb::RC & rc ) { if( rc != SUCCESS ) { std::cerr << "\t Sparse-mask set (re-entrance) FAILED with error code " << grb::toString( rc ) << "\n"; } else { - if( nnz( dst ) != 2 ) { + if( nnz( dst ) != 1 ) { std::cerr << "\t (sparse-mask-set-reentrant) unexpected number of nonzeroes " << nnz( dst ) << ", expected 1.\n"; rc = FAILED; } @@ -266,7 +266,7 @@ void grb_program( const size_t & n, grb::RC & rc ) { if( rc != SUCCESS ) { std::cerr << "\t Sparse-mask set to scalar (re-entrant) FAILED with error code " << grb::toString( rc ) << "\n"; } else { - if( nnz( dst ) != 2 ) { + if( nnz( dst ) != 1 ) { std::cerr << "\t (sparse-mask-set-scalar-reentrant) unexpected number of nonzeroes " << nnz( dst ) << ", expected 1.\n"; rc = FAILED; } From 90f3d36831b87b3d72f09c3d12deb6033a2dcbf4 Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Fri, 14 Oct 2022 18:29:37 +0200 Subject: [PATCH 02/14] Fix unrelated typo --- tests/smoke/smoketests.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/smoke/smoketests.sh b/tests/smoke/smoketests.sh index 576356f61..5965329d2 100755 --- a/tests/smoke/smoketests.sh +++ b/tests/smoke/smoketests.sh @@ -202,7 +202,7 @@ for BACKEND in ${BACKENDS[@]}; do if [ -f ${TEST_DATA_DIR}/${TESTNAME}.mtx ]; then n=$(grep -v '^%' ${TEST_DATA_DIR}/${TESTNAME}.mtx | head -1 | awk '{print $1}' ) m=$(grep -v '^%' ${TEST_DATA_DIR}/${TESTNAME}.mtx | head -1 | awk '{print $2}' ) - echo ">>> [x] [ ] Testing the conjugate gradient complex algorithm for the input" + echo ">>> [x] [ ] Testing the conjugate gradient complex algorithm for the input" echo " matrix (${n}x${m}) taken from ${TESTNAME}.mtx. This test" echo " verifies against a ground-truth solution vector. The test" echo " employs the grb::Launcher in automatic mode. It uses" From d870ea7916c7593ad92ccae8b90a1dc731d4452b Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Fri, 14 Oct 2022 18:30:50 +0200 Subject: [PATCH 03/14] Bugfix: if vectors are dense but the mask is not, the generic fold from vectors into vectors still handles it appropriately. This, however, means that the semantic check for the dense descriptor cannot be handled generically. --- include/graphblas/reference/blas1.hpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/include/graphblas/reference/blas1.hpp b/include/graphblas/reference/blas1.hpp index baf93873e..16c810663 100644 --- a/include/graphblas/reference/blas1.hpp +++ b/include/graphblas/reference/blas1.hpp @@ -1058,9 +1058,6 @@ namespace grb { if( !sparse && nnz( to_fold ) < n ) { return ILLEGAL; } - if( masked && !sparse && nnz( *m ) < n ) { - return ILLEGAL; - } if( phase == RESIZE ) { return SUCCESS; } From fbde1413b6fc1c41c9026d6b75ce30f71e4d18cf Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Fri, 14 Oct 2022 18:40:28 +0200 Subject: [PATCH 04/14] Lift out the check for dense descriptor for vector-to-vector folds --- include/graphblas/reference/blas1.hpp | 31 +++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/include/graphblas/reference/blas1.hpp b/include/graphblas/reference/blas1.hpp index 16c810663..ab31e6e3a 100644 --- a/include/graphblas/reference/blas1.hpp +++ b/include/graphblas/reference/blas1.hpp @@ -1849,6 +1849,12 @@ namespace grb { return MISMATCH; } + const size_t n = size( x ); + + if( descr & descriptors::dense ) { + if( nnz( x ) != n || nnz( y ) != n ) { return ILLEGAL; } + } + #ifdef _DEBUG std::cout << "In foldr ([T]<-[T])\n"; #endif @@ -1916,6 +1922,10 @@ namespace grb { if( n != size( y ) || n != size( m ) ) { return MISMATCH; } + if( descr & descriptors::dense ) { + if( size( m ) > 0 && nnz( m ) != n ) { return ILLEGAL; } + if( nnz( x ) != n || nnz( y ) != n ) { return ILLEGAL; } + } if( nnz( x ) < n || nnz( y ) < n ) { return internal::fold_from_vector_to_vector_generic< @@ -2033,6 +2043,9 @@ namespace grb { if( n != size( y ) ) { return MISMATCH; } + if( descr & descriptors::dense ) { + if( nnz( x ) != n || nnz( y ) != n ) { return ILLEGAL; } + } const Vector< bool, reference, Coords > * const null_mask = nullptr; if( nnz( x ) < n || nnz( y ) < n ) { @@ -2098,6 +2111,10 @@ namespace grb { if( n != size( y ) || n != size( m ) ) { return MISMATCH; } + if( descr & descriptors::dense ) { + if( size( m ) > 0 && nnz( m ) != n ) { return ILLEGAL; } + if( nnz( x ) != n || nnz( y ) != n ) { return ILLEGAL; } + } if( nnz( x ) < n || nnz( y ) < n ) { return internal::fold_from_vector_to_vector_generic< @@ -2577,6 +2594,9 @@ namespace grb { if( n != size( y ) ) { return MISMATCH; } + if( descr & descriptors::dense ) { + if( nnz( x ) != n || nnz( y ) != n ) { return ILLEGAL; } + } // all OK, execute const Vector< bool, reference, Coords > * const null_mask = nullptr; @@ -2700,6 +2720,9 @@ namespace grb { if( n != size( y ) ) { return MISMATCH; } + if( descr & descriptors::dense ) { + if( nnz( x ) != n || nnz( y ) != n ) { return ILLEGAL; } + } // all OK, execute const Vector< bool, reference, Coords > * const null_mask = nullptr; @@ -2768,6 +2791,10 @@ namespace grb { if( n != size( y ) || n != size( m ) ) { return MISMATCH; } + if( descr & descriptors::dense ) { + if( size( m ) > 0 && nnz( m ) != n ) { return ILLEGAL; } + if( nnz( x ) != n || nnz( y ) != n ) { return ILLEGAL; } + } // all OK, execute if( nnz( x ) < n || nnz( y ) < n ) { @@ -2835,6 +2862,10 @@ namespace grb { if( n != size( y ) || n != size( m ) ) { return MISMATCH; } + if( descr & descriptors::dense ) { + if( size( m ) > 0 && nnz( m ) != n ) { return ILLEGAL; } + if( nnz( x ) != n || nnz( y ) != n ) { return ILLEGAL; } + } // all OK, execute if( nnz( x ) < n || nnz( y ) < n ) { From 8eff995f539a47c03b4c749299d08ce89b5b766b Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Fri, 14 Oct 2022 18:41:05 +0200 Subject: [PATCH 05/14] Bugfix: implement missing foldl (masked vector-to-vector) in BSP1D backend. Also minor code style fix. --- include/graphblas/bsp1d/blas1.hpp | 178 +++++++++++++++++++++++++++++- 1 file changed, 175 insertions(+), 3 deletions(-) diff --git a/include/graphblas/bsp1d/blas1.hpp b/include/graphblas/bsp1d/blas1.hpp index 51d25a96e..ac7bb3514 100644 --- a/include/graphblas/bsp1d/blas1.hpp +++ b/include/graphblas/bsp1d/blas1.hpp @@ -657,6 +657,178 @@ namespace grb { return ret; } + /** \internal No implementation notes */ + template< + Descriptor descr = descriptors::no_operation, class OP, + typename IOType, typename MaskType, typename InputType, + typename Coords + > + RC foldl( + Vector< IOType, BSP1D, Coords > &x, + const Vector< MaskType, BSP1D, Coords > &m, + const Vector< InputType, BSP1D, Coords > &y, + const OP &op = OP(), + const Phase &phase = EXECUTE, + const typename std::enable_if< grb::is_operator< OP >::value && + !grb::is_object< IOType >::value && + !grb::is_object< MaskType >::value && + !grb::is_object< InputType >::value, void + >::type * = nullptr + ) { + // static sanity checks + NO_CAST_ASSERT( ( !(descr & descriptors::no_casting) || + std::is_same< typename OP::D1, IOType >::value ), + "grb::foldl", + "called with a vector x of a type that does not match the first domain " + "of the given operator" ); + NO_CAST_ASSERT( ( !(descr & descriptors::no_casting) || + std::is_same< typename OP::D2, InputType >::value ), + "grb::foldl", + "called on a vector y of a type that does not match the second domain " + "of the given operator" ); + NO_CAST_ASSERT( ( !(descr & descriptors::no_casting) || + std::is_same< typename OP::D3, IOType >::value ), + "grb::foldl", + "called on a vector x of a type that does not match the third domain " + "of the given operator" ); + NO_CAST_ASSERT( ( !(descr & descriptors::no_casting) || + std::is_same< bool, MaskType >::value ), + "grb::foldl", + "called with a mask that does not have boolean entries " ); + + // catch empty mask + if( size( m ) == 0 ) { + return foldl< descr >( x, y, op, phase ); + } + + // dynamic sanity checks + const size_t n = size( x ); + if( n != size( y ) || n != size( m ) ) { + return MISMATCH; + } + + // handle trivial resize phase + if( config::IMPLEMENTATION< BSP1D >::fixedVectorCapacities() && + phase == RESIZE + ) { + return SUCCESS; + } + + // delegate + RC ret = foldl< descr >( + internal::getLocal( x ), internal::getLocal( m ), + internal::getLocal( y ), + op, phase + ); + + if( !config::IMPLEMENTATION< BSP1D >::fixedVectorCapacities() ) { + if( collectives< BSP1D >::allreduce( + ret, grb::operators::any_or< RC >() + ) != SUCCESS ) { + return PANIC; + } + } + + // handle try and execute phases + if( phase != RESIZE ) { + if( ret == SUCCESS ) { + ret = internal::updateNnz( x ); + } else if( ret == FAILED ) { + const RC subrc = internal::updateNnz( x ); + if( subrc != SUCCESS ) { ret = PANIC; } + } + } + + // done + return ret; + } + + /** \internal No implementation notes */ + template< + Descriptor descr = descriptors::no_operation, class Monoid, + typename IOType, typename MaskType, typename InputType, + typename Coords + > + RC foldl( + Vector< IOType, BSP1D, Coords > &x, + const Vector< MaskType, BSP1D, Coords > &m, + const Vector< InputType, BSP1D, Coords > &y, + const Monoid &monoid = Monoid(), + const Phase &phase = EXECUTE, + const typename std::enable_if< grb::is_monoid< Monoid >::value && + !grb::is_object< IOType >::value && + !grb::is_object< MaskType >::value && + !grb::is_object< InputType >::value, void + >::type * = nullptr + ) { + // static sanity checks + NO_CAST_ASSERT( ( !(descr & descriptors::no_casting) || + std::is_same< typename Monoid::D1, IOType >::value ), + "grb::foldl", + "called with a vector x of a type that does not match the first domain " + "of the given operator" ); + NO_CAST_ASSERT( ( !(descr & descriptors::no_casting) || + std::is_same< typename Monoid::D2, InputType >::value ), + "grb::foldl", + "called on a vector y of a type that does not match the second domain " + "of the given operator" ); + NO_CAST_ASSERT( ( !(descr & descriptors::no_casting) || + std::is_same< typename Monoid::D3, IOType >::value ), + "grb::foldl", + "called on a vector x of a type that does not match the third domain " + "of the given operator" ); + NO_CAST_ASSERT( ( !(descr & descriptors::no_casting) || + std::is_same< bool, MaskType >::value ), + "grb::foldl", + "called with a mask that does not have boolean entries" ); + + // catch empty mask + if( size( m ) == 0 ) { + return foldl< descr >( x, y, monoid, phase ); + } + + // dynamic sanity checks + const size_t n = size( x ); + if( n != size( y ) || n != size( m ) ) { + return MISMATCH; + } + + // handle trivial resize phase + if( config::IMPLEMENTATION< BSP1D >::fixedVectorCapacities() && + phase == RESIZE + ) { + return SUCCESS; + } + + // delegate + RC ret = foldl< descr >( + internal::getLocal( x ), internal::getLocal( m ), + internal::getLocal( y ), + monoid, phase + ); + + if( !config::IMPLEMENTATION< BSP1D >::fixedVectorCapacities() ) { + if( collectives< BSP1D >::allreduce( + ret, grb::operators::any_or< RC >() + ) != SUCCESS ) { + return PANIC; + } + } + + // handle try and execute phases + if( phase != RESIZE ) { + if( ret == SUCCESS ) { + ret = internal::updateNnz( x ); + } else if( ret == FAILED ) { + const RC subrc = internal::updateNnz( x ); + if( subrc != SUCCESS ) { ret = PANIC; } + } + } + + // done + return ret; + } + /** \internal No communication necessary, output is guaranteed dense. */ template< Descriptor descr = descriptors::no_operation, @@ -683,17 +855,17 @@ namespace grb { // static checks NO_CAST_ASSERT( ( !(descr & descriptors::no_casting) || - std::is_same< InputType1, typename Operator::D1 >::value ), + std::is_same< InputType1, typename Operator::D1 >::value ), "grb::eWiseApply", "called with a left-hand input vector value type that does not match the " "first domain of the given operator " ); NO_CAST_ASSERT( ( !(descr & descriptors::no_casting) || - std::is_same< InputType2, typename Operator::D2 >::value ), + std::is_same< InputType2, typename Operator::D2 >::value ), "grb::eWiseApply", "called with a right-hand input vector value type that does not match the second " "domain of the given operator" ); NO_CAST_ASSERT( ( !(descr & descriptors::no_casting) || - std::is_same< OutputType, typename Operator::D3 >::value ), + std::is_same< OutputType, typename Operator::D3 >::value ), "grb::eWiseApply", "called with an output value type that does not match the third domain of " "the given operator" ); From b7d0cab06b66902f2432f2b05160a7b087f23202 Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Fri, 14 Oct 2022 18:41:49 +0200 Subject: [PATCH 06/14] Bugfix: label propagation was using a masked grb::set while expecting in-place semantics. --- include/graphblas/algorithms/label.hpp | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/include/graphblas/algorithms/label.hpp b/include/graphblas/algorithms/label.hpp index cfebc824f..7bb5e0a75 100644 --- a/include/graphblas/algorithms/label.hpp +++ b/include/graphblas/algorithms/label.hpp @@ -116,7 +116,8 @@ namespace grb { * accelerating the PageRank computation', ACM Press, 2003. */ template< typename IOType > - RC label( Vector< IOType > &out, + RC label( + Vector< IOType > &out, const Vector< IOType > &y, const Matrix< IOType > &W, const size_t n, const size_t l, const size_t MaxIterations = 1000 @@ -230,7 +231,12 @@ namespace grb { << "nnz( mask ) = " << nnz( mask ) << "\n"; #endif // clamps the first l labelled nodes - ret = ret ? ret : set( fNext, mask, f ); + ret = ret ? ret : foldl( + fNext, mask, + f, + grb::operators::right_assign< IOType >() + ); + assert( ret == SUCCESS ); #ifdef _DEBUG std::cerr << "\t post-set nnz( fNext ) = " << nnz( fNext ) << "\n"; printVector( @@ -246,28 +252,31 @@ namespace grb { #ifdef _DEBUG std::cerr << "\t pre-set nnz(f) = " << nnz( f ) << "\n"; #endif - ret = ret ? ret : set( f, fNext ); + std::swap( f, fNext ); #ifdef _DEBUG std::cerr << "\t post-set nnz(f) = " << nnz( f ) << "\n"; #endif // go to next iteration - (void)++iter; + (void) ++iter; } if( ret == SUCCESS ) { if( different ) { if( s == 0 ) { - std::cout << "Warning: label propagation did not converge after " + std::cerr << "Info: label propagation did not converge after " << (iter-1) << " iterations\n"; } return FAILED; } else { if( s == 0 ) { - std::cout << "Info: label propagation converged in " + std::cerr << "Info: label propagation converged in " << (iter-1) << " iterations\n"; } - return set( out, f ); + std::swap( out, f ); + return SUCCESS; } + } else { + std::cerr << "Info: label propagation exiting with " << toString(ret) << "\n"; } // done From a171bc5bfff37016f80ea676389a64cff122ecbf Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Fri, 14 Oct 2022 18:42:17 +0200 Subject: [PATCH 07/14] Code style fixes and better output to stdout/stderr --- tests/smoke/label.cpp | 37 +++++++++++++++++++--------------- tests/smoke/label_test.cpp | 41 ++++++++++++++++++++++---------------- 2 files changed, 45 insertions(+), 33 deletions(-) diff --git a/tests/smoke/label.cpp b/tests/smoke/label.cpp index fb24c1625..f99c18d91 100644 --- a/tests/smoke/label.cpp +++ b/tests/smoke/label.cpp @@ -41,7 +41,10 @@ using namespace grb; constexpr size_t MaxPrinting = 10; // forward declaration of the graph dataset parser -bool readEdges( std::string filename, bool use_indirect, size_t * n, size_t * nz, size_t ** I, size_t ** J, double ** weights ); +bool readEdges( + std::string filename, bool use_indirect, + size_t * n, size_t * nz, size_t ** I, size_t ** J, double ** weights +); struct input { char filename[ 1024 ]; @@ -186,10 +189,9 @@ int main( int argc, char ** argv ) { // sanity check if( argc < 3 || argc > 5 ) { - std::cout << "Usage: " << argv[ 0 ] - << " (number of inner " - "iterations) (number of outer iterations)" - << std::endl; + std::cout << "Usage: " << argv[ 0 ] << " " + << "(number of inner iterations) (number of outer iterations)" + << std::endl; return 0; } std::cout << "Test executable: " << argv[ 0 ] << std::endl; @@ -200,7 +202,7 @@ int main( int argc, char ** argv ) { std::cerr << "Could not parse filename: too long." << std::endl; return 10; } - (void)strncpy( in.filename, argv[ 1 ], 1023 ); + (void) strncpy( in.filename, argv[ 1 ], 1023 ); in.filename[ 1023 ] = '\0'; if( strncmp( argv[ 2 ], "direct", 6 ) == 0 ) { in.direct = true; @@ -217,23 +219,24 @@ int main( int argc, char ** argv ) { if( argc >= 4 ) { inner = strtoumax( argv[ 3 ], &end, 10 ); if( argv[ 3 ] == end ) { - std::cerr << "Could not parse argument for number of inner " - "repititions." - << std::endl; + std::cerr << "Could not parse argument for number of inner repititions." + << std::endl; return 30; } } if( argc >= 5 ) { outer = strtoumax( argv[ 4 ], &end, 10 ); if( argv[ 4 ] == end ) { - std::cerr << "Could not parse argument for number of outer " - "repititions." - << std::endl; + std::cerr << "Could not parse argument for number of outer repititions." + << std::endl; return 40; } } - std::cout << "Executable called with parameters filename " << in.filename << ", direct = " << in.direct << ", and #vertices = " << in.n << std::endl; + std::cout << "Executable called with parameters " + << "filename " << in.filename << ", " + << "direct = " << in.direct << ", and " + << "#vertices = " << in.n << std::endl; // the output struct struct output out; @@ -242,7 +245,8 @@ int main( int argc, char ** argv ) { enum grb::RC rc = launcher.exec( &grbProgram, in, out, inner, outer, true ); if( rc != SUCCESS ) { - std::cerr << "launcher.exec returns with non-SUCCESS error code " << (int)rc << std::endl; + std::cerr << "launcher.exec returns with non-SUCCESS error code " + << toString(rc) << std::endl; return 50; } @@ -250,9 +254,10 @@ int main( int argc, char ** argv ) { // done if( out.error_code != SUCCESS ) { - std::cout << "Test FAILED.\n\n"; + std::cout << "Test FAILED\n\n"; return 255; } - std::cout << "Test OK.\n\n"; + std::cout << "Test OK\n\n"; return 0; } + diff --git a/tests/smoke/label_test.cpp b/tests/smoke/label_test.cpp index f8374fdcd..246b31e53 100644 --- a/tests/smoke/label_test.cpp +++ b/tests/smoke/label_test.cpp @@ -34,6 +34,7 @@ #include "graphblas.hpp" + using namespace grb; static size_t n; // total number of vertices @@ -62,13 +63,15 @@ using namespace grb; bool init_input( const struct input & data_in ) { // a binary tree with data_in.n vertices at the leaves n = 2 * data_in.n - 1; - // construct the input labels: the first l (leaves) are clamped to 1 and the rest are 0 + // construct the input labels: the first l (leaves) are clamped to 1 and the + // rest are 0 l = data_in.n; labels = new double[ n ]; for( size_t i = 0; i < n; i++ ) labels[ i ] = ( i < l ) ? 1 : 0; - // there are n-1 edges in the tree and so 2(n-1) in the matrix, since it's a symmetric + // there are n-1 edges in the tree and so 2(n-1) in the matrix, since it's + // symmetric nz = 2 * ( n - 1 ); weights = new double[ nz ]; I = new size_t[ nz ]; @@ -82,13 +85,11 @@ bool init_input( const struct input & data_in ) { size_t dst = ( e & ~0x01 ) + pow( 2.0, ( levels - level ) ) - floor( edge / 2 ); I[ e ] = e; J[ e ] = dst; - // std::cout << "e " << e << " level " << level << " edge " << edge << " edges " << edges << " I " << I[e] << " J " << J[e] << std::endl; weights[ e ] = 1.0; size_t e_other = e + ( nz / 2 ); I[ e_other ] = dst; J[ e_other ] = e; weights[ e_other ] = 1.0; - // std::cout << "e other " << e_other << " level " << level << " edge " << edge << " edges " << edges << " I " << I[e_other] << " J " << J[e_other] << std::endl; edge++; // update counters when we come to the end of the current tree level if( edge == edges ) { @@ -110,12 +111,14 @@ void free_input() { } // main label propagation algorithm -void grbProgram( const struct input & data_in, struct output & out ) { +void grbProgram( const struct input &data_in, struct output &out ) { grb::utils::Timer timer; timer.reset(); const size_t s = spmd<>::pid(); - (void)s; +#ifdef NDEBUG + (void) s; +#endif assert( s < spmd<>::nprocs() ); // get input n @@ -129,12 +132,15 @@ void grbProgram( const struct input & data_in, struct output & out ) { // create the intial set of l input labels in the vector y Vector< double > y( n ); Vector< double > f( n ); - buildVector( y, &( labels[ 0 ] ), &( labels[ 0 ] ) + n, SEQUENTIAL ); + buildVector( y, &(labels[ 0 ]), &(labels[ 0 ]) + n, SEQUENTIAL ); // create the symmetric weight matrix W, representing the weighted graph Matrix< double > W( n, n ); resize( W, nz ); - RC rc = buildMatrixUnique( W, &( I[ 0 ] ), &( J[ 0 ] ), &( weights[ 0 ] ), nz, SEQUENTIAL ); + RC rc = buildMatrixUnique( + W, &(I[ 0 ]), &(J[ 0 ]), &(weights[ 0 ]), nz, + SEQUENTIAL + ); if( rc != SUCCESS ) { out.error_code = ILLEGAL; free_input(); @@ -156,10 +162,9 @@ void grbProgram( const struct input & data_in, struct output & out ) { int main( int argc, char ** argv ) { // sanity check if( argc < 2 || argc > 4 ) { - std::cout << "Usage: " << argv[ 0 ] - << " (number of inner iterations) " - "(number of outer iterations)" - << std::endl; + std::cout << "Usage: " << argv[ 0 ] << " " + << "(number of inner iterations) (number of outer iterations)" + << std::endl; return 0; } std::cout << "Test executable: " << argv[ 0 ] << std::endl; @@ -167,7 +172,7 @@ int main( int argc, char ** argv ) { // the input struct struct input in; in.n = atoi( argv[ 1 ] ); - std::cout << "Executable called with parameters #vertices = " << in.n << std::endl; + std::cout << "Executable called with parameters #vertices = " << in.n << "\n"; // the output struct struct output out; @@ -176,17 +181,19 @@ int main( int argc, char ** argv ) { enum grb::RC rc = launcher.exec( &grbProgram, in, out ); if( rc != SUCCESS ) { - std::cerr << "launcher.exec returns with non-SUCCESS error code " << (int)rc << std::endl; + std::cerr << "launcher.exec returns with non-SUCCESS error code " + << toString(rc) << std::endl; return 50; } - std::cout << "Error code is " << out.error_code << ".\n"; + std::cout << "Error code is " << toString(out.error_code) << "\n"; // done if( out.error_code != SUCCESS ) { - std::cout << "Test FAILED.\n\n"; + std::cout << "Test FAILED\n\n"; return 1; } - std::cout << "Test OK.\n\n"; + std::cout << "Test OK\n\n"; return 0; } + From 3544996e6db041d875eebbecd3d10031f74f85ad Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Sat, 15 Oct 2022 12:54:16 +0200 Subject: [PATCH 08/14] Match the return to the related output, and only print the output code once in case of multiple user processes --- include/graphblas/algorithms/label.hpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/include/graphblas/algorithms/label.hpp b/include/graphblas/algorithms/label.hpp index 7bb5e0a75..a20e47191 100644 --- a/include/graphblas/algorithms/label.hpp +++ b/include/graphblas/algorithms/label.hpp @@ -275,11 +275,13 @@ namespace grb { std::swap( out, f ); return SUCCESS; } - } else { - std::cerr << "Info: label propagation exiting with " << toString(ret) << "\n"; } // done + if( s == 0 ) { + std::cerr << "Warning: label propagation exiting with " << toString(ret) + << "\n"; + } return ret; } From b0bfa6a8594c5e53b52a90264cb20d553163f432 Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Sat, 15 Oct 2022 13:13:52 +0200 Subject: [PATCH 09/14] Code review in progress, comitting since not sure will continue from this machine -- apologies --- tests/unit/set.cpp | 80 ++++++++++++++++++++++++++++++---------------- 1 file changed, 53 insertions(+), 27 deletions(-) diff --git a/tests/unit/set.cpp b/tests/unit/set.cpp index 6f9f0919b..7c11177e9 100644 --- a/tests/unit/set.cpp +++ b/tests/unit/set.cpp @@ -22,7 +22,7 @@ using namespace grb; -void grb_program( const size_t & n, grb::RC & rc ) { +void grb_program( const size_t &n, grb::RC &rc ) { grb::Vector< double > dst( n ), src( n ); // test set @@ -31,12 +31,15 @@ void grb_program( const size_t & n, grb::RC & rc ) { std::cerr << "\tset-to-value FAILED\n"; } else { if( nnz( src ) != n ) { - std::cerr << "\t (set-to-value) unexpected number of nonzeroes " << nnz( src ) << ", expected " << n << "\n"; + std::cerr << "\t (set-to-value) unexpected number of nonzeroes " + << nnz( src ) << ", expected " << n << "\n"; rc = FAILED; } - for( const auto & pair : src ) { + for( const auto &pair : src ) { if( pair.second != 1.5 ) { - std::cerr << "\t (set-to-value) unexpected entry ( " << pair.first << ", " << pair.second << " ), expected value 1.5.\n"; + std::cerr << "\t (set-to-value) unexpected entry " + << ( " << pair.first << ", " << pair.second << " ), " + << "expected value 1.5.\n"; rc = FAILED; } } @@ -51,12 +54,15 @@ void grb_program( const size_t & n, grb::RC & rc ) { std::cerr << "\tset-to-index FAILED\n"; } else { if( nnz( dst ) != n ) { - std::cerr << "\t (set-to-index) unexpected number of nonzeroes " << nnz( dst ) << ", expected " << n << "\n"; + std::cerr << "\t (set-to-index) unexpected number of nonzeroes " + << nnz( dst ) << ", expected " << n << "\n"; rc = FAILED; } - for( const auto & pair : dst ) { + for( const auto &pair : dst ) { if( pair.second != pair.first ) { - std::cerr << "\t (set-to-index) unexpected entry ( " << pair.first << ", " << pair.second << " ), expected value " << static_cast< double >( pair.first ) << ".\n"; + std::cerr << "\t (set-to-index) unexpected entry " + << "( " << pair.first << ", " << pair.second << " ), " + << "expected value " << static_cast< double >( pair.first ) << ".\n"; rc = FAILED; } } @@ -68,15 +74,19 @@ void grb_program( const size_t & n, grb::RC & rc ) { // test set overwrite rc = grb::set( dst, src ); if( rc != SUCCESS ) { - std::cerr << "\t Set-overwrite FAILED with error code " << grb::toString( rc ) << "\n"; + std::cerr << "\t Set-overwrite FAILED with error code " + << grb::toString( rc ) << "\n"; } else { if( nnz( dst ) != n ) { - std::cerr << "\t (set-overwrite) unexpected number of nonzeroes " << nnz( dst ) << ", expected " << n << "\n"; + std::cerr << "\t (set-overwrite) unexpected number of nonzeroes " + << nnz( dst ) << ", expected " << n << "\n"; rc = FAILED; } - for( const auto & pair : dst ) { + for( const auto &pair : dst ) { if( pair.second != 1.5 ) { - std::cerr << "\t (set-overwrite) unexpected entry ( " << pair.first << ", " << pair.second << " ), expected value 1.5\n"; + std::cerr << "\t (set-overwrite) unexpected entry " + << "( " << pair.first << ", " << pair.second << " ), " + << "expected value 1.5\n"; rc = FAILED; } } @@ -91,15 +101,19 @@ void grb_program( const size_t & n, grb::RC & rc ) { rc = grb::set( dst, src ); } if( rc != SUCCESS ) { - std::cerr << "\t Set-into-cleared FAILED with error code " << grb::toString( rc ) << "\n"; + std::cerr << "\t Set-into-cleared FAILED with error code " + << grb::toString( rc ) << "\n"; } else { if( nnz( dst ) != n ) { - std::cerr << "\t (set-into-cleared) unexpected number of nonzeroes " << nnz( dst ) << ", expected " << n << "\n"; + std::cerr << "\t (set-into-cleared) unexpected number of nonzeroes " + << nnz( dst ) << ", expected " << n << "\n"; rc = FAILED; } - for( const auto & pair : dst ) { + for( const auto &pair : dst ) { if( pair.second != 1.5 ) { - std::cerr << "\t (set-into-cleared) unexpected entry ( " << pair.first << ", " << pair.second << " ), expected value 1.5\n"; + std::cerr << "\t (set-into-cleared) unexpected entry " + << "( " << pair.first << ", " << pair.second << " ), " + << "expected value 1.5\n"; rc = FAILED; } } @@ -117,19 +131,25 @@ void grb_program( const size_t & n, grb::RC & rc ) { rc = grb::set( dst, src, src ); } if( rc != SUCCESS ) { - std::cerr << "\t Masked-set FAILED with error code " << grb::toString( rc ) << "\n"; + std::cerr << "\t Masked-set FAILED with error code " + << grb::toString( rc ) << "\n"; } else { if( nnz( dst ) != n - 1 ) { - std::cerr << "\t (masked-set) unexpected number of nonzeroes " << nnz( dst ) << ", expected " << ( n - 1 ) << "\n"; + std::cerr << "\t (masked-set) unexpected number of nonzeroes " + << nnz( dst ) << ", expected " << ( n - 1 ) << "\n"; rc = FAILED; } - for( const auto & pair : dst ) { + for( const auto &pair : dst ) { if( pair.first != n / 2 && pair.second != 1.5 ) { - std::cerr << "\t (masked-set) unexpected entry ( " << pair.first << ", " << pair.second << " ), expected value 1.5\n"; + std::cerr << "\t (masked-set) unexpected entry " + << "( " << pair.first << ", " << pair.second << " ), " + << "expected value 1.5\n"; rc = FAILED; } if( pair.first == n / 2 ) { - std::cerr << "\t (masked-set) unexpected entry ( " << pair.first << ", " << pair.second << " ), expected no entry at this position\n"; + std::cerr << "\t (masked-set) unexpected entry " + << "( " << pair.first << ", " << pair.second << " ), " + << "expected no entry at this position\n"; rc = FAILED; } } @@ -144,21 +164,25 @@ void grb_program( const size_t & n, grb::RC & rc ) { rc = grb::set< grb::descriptors::invert_mask >( dst, src, src ); } if( rc != SUCCESS ) { - std::cerr << "\t Inverted-mask set FAILED with error code " << grb::toString( rc ) << "\n"; + std::cerr << "\t Inverted-mask set FAILED with error code " + << grb::toString( rc ) << "\n"; } else { if( nnz( dst ) != 1 ) { - std::cerr << "\t (inverted-mask-set) unexpected number of " - "nonzeroes " - << nnz( dst ) << ", expected 1.\n"; + std::cerr << "\t (inverted-mask-set) unexpected number of nonzeroes " + << nnz( dst ) << ", expected 1.\n"; rc = FAILED; } - for( const auto & pair : dst ) { + for( const auto &pair : dst ) { if( pair.first == n / 2 && pair.second != 0 ) { - std::cerr << "\t (inverted-mask-set) unexpected entry ( " << pair.first << ", " << pair.second << ": expected value 0\n"; + std::cerr << "\t (inverted-mask-set) unexpected entry " + << "( " << pair.first << ", " << pair.second << " ), " + << "expected value 0\n"; rc = FAILED; } if( pair.first != n / 2 ) { - std::cerr << "\t (inverted-mask-set) unexpected entry ( " << pair.first << ", " << pair.second << " ): expected no entry at this position\n"; + std::cerr << "\t (inverted-mask-set) unexpected entry " + << "( " << pair.first << ", " << pair.second << " ), " + << "expected no entry at this position\n"; rc = FAILED; } } @@ -167,6 +191,8 @@ void grb_program( const size_t & n, grb::RC & rc ) { return; } + // TODO continue code review from here. Also check if there is a set-to-empty test + // test sparse mask set rc = grb::clear( dst ); if( rc == SUCCESS ) { From 0400286ca25fddabe1796fc20e5f37101d8199c0 Mon Sep 17 00:00:00 2001 From: Albert-Jan Yzelman Date: Sun, 16 Oct 2022 12:13:54 +0200 Subject: [PATCH 10/14] Code review set unit test --- tests/unit/set.cpp | 158 +++++++++++++++++++++++++++------------------ 1 file changed, 94 insertions(+), 64 deletions(-) diff --git a/tests/unit/set.cpp b/tests/unit/set.cpp index 7c11177e9..2ba74d4e1 100644 --- a/tests/unit/set.cpp +++ b/tests/unit/set.cpp @@ -20,6 +20,7 @@ #include + using namespace grb; void grb_program( const size_t &n, grb::RC &rc ) { @@ -38,7 +39,7 @@ void grb_program( const size_t &n, grb::RC &rc ) { for( const auto &pair : src ) { if( pair.second != 1.5 ) { std::cerr << "\t (set-to-value) unexpected entry " - << ( " << pair.first << ", " << pair.second << " ), " + << "( " << pair.first << ", " << pair.second << " ), " << "expected value 1.5.\n"; rc = FAILED; } @@ -124,9 +125,6 @@ void grb_program( const size_t &n, grb::RC &rc ) { // test masked set rc = grb::setElement( src, 0, n / 2 ); - if( rc == SUCCESS ) { - rc = grb::clear( dst ); - } if( rc == SUCCESS ) { rc = grb::set( dst, src, src ); } @@ -159,10 +157,7 @@ void grb_program( const size_t &n, grb::RC &rc ) { } // test inverted-mask set - rc = grb::clear( dst ); - if( rc == SUCCESS ) { - rc = grb::set< grb::descriptors::invert_mask >( dst, src, src ); - } + rc = grb::set< grb::descriptors::invert_mask >( dst, src, src ); if( rc != SUCCESS ) { std::cerr << "\t Inverted-mask set FAILED with error code " << grb::toString( rc ) << "\n"; @@ -191,13 +186,8 @@ void grb_program( const size_t &n, grb::RC &rc ) { return; } - // TODO continue code review from here. Also check if there is a set-to-empty test - // test sparse mask set - rc = grb::clear( dst ); - if( rc == SUCCESS ) { - rc = grb::clear( src ); - } + rc = grb::clear( src ); if( rc == SUCCESS ) { rc = grb::setElement( src, 1.5, n / 2 ); } @@ -205,19 +195,25 @@ void grb_program( const size_t &n, grb::RC &rc ) { rc = grb::set( dst, src, src ); } if( rc != SUCCESS ) { - std::cerr << "\t Sparse-mask set FAILED with error code " << grb::toString( rc ) << "\n"; + std::cerr << "\t Sparse-mask set FAILED with error code " + << grb::toString( rc ) << "\n"; } else { if( nnz( dst ) != 1 ) { - std::cerr << "\t (sparse-mask-set) unexpected number of nonzeroes " << nnz( dst ) << ", expected 1.\n"; + std::cerr << "\t (sparse-mask-set) unexpected number of nonzeroes " + << nnz( dst ) << ", expected 1.\n"; rc = FAILED; } - for( const auto & pair : dst ) { + for( const auto &pair : dst ) { if( pair.first == n / 2 && pair.second != 1.5 ) { - std::cerr << "\t (sparse-mask-set) unexpected entry ( " << pair.first << ", " << pair.second << " ): expected value 1.5\n"; + std::cerr << "\t (sparse-mask-set) unexpected entry " + << "( " << pair.first << ", " << pair.second << " ), " + << "expected value 1.5\n"; rc = FAILED; } if( pair.first != n / 2 ) { - std::cerr << "\t (sparse-mask-set) unexpected entry ( " << pair.first << ", " << pair.second << " ): expected no entry at this position\n"; + std::cerr << "\t (sparse-mask-set) unexpected entry " + << "( " << pair.first << ", " << pair.second << " ), " + << "expected no entry at this position\n"; rc = FAILED; } } @@ -231,19 +227,25 @@ void grb_program( const size_t &n, grb::RC &rc ) { rc = rc ? rc : grb::setElement( src, 1.5, 0 ); rc = rc ? rc : grb::set( dst, src, src ); if( rc != SUCCESS ) { - std::cerr << "\t Sparse-mask set (re-entrance) FAILED with error code " << grb::toString( rc ) << "\n"; + std::cerr << "\t Sparse-mask set (re-entrance) FAILED with error code " + << grb::toString( rc ) << "\n"; } else { if( nnz( dst ) != 1 ) { - std::cerr << "\t (sparse-mask-set-reentrant) unexpected number of nonzeroes " << nnz( dst ) << ", expected 1.\n"; + std::cerr << "\t (sparse-mask-set-reentrant) unexpected number of nonzeroes " + << nnz( dst ) << ", expected 1.\n"; rc = FAILED; } for( const auto &pair : dst ) { - if( (pair.first == 0 || pair.first == n/2) && pair.second != 1.5 ) { - std::cerr << "\t (sparse-mask-set-reentrant) unexpected entry ( " << pair.first << ", " << pair.second << " ): expected value 1.5\n"; + if( pair.first == 0 && pair.second != 1.5 ) { + std::cerr << "\t (sparse-mask-set-reentrant) unexpected entry " + << "( " << pair.first << ", " << pair.second << " ), " + << "expected value 1.5\n"; rc = FAILED; } - if( pair.first != 0 && pair.first != n/2 ) { - std::cerr << "\t (sparse-mask-set-reentrant) unexpected entry ( " << pair.first << ", " << pair.second << " ): expected no entry at this position\n"; + if( pair.first != 0 ) { + std::cerr << "\t (sparse-mask-set-reentrant) unexpected entry " + << "( " << pair.first << ", " << pair.second << " ), " + << "expected no entry at this position\n"; rc = FAILED; } } @@ -253,10 +255,7 @@ void grb_program( const size_t &n, grb::RC &rc ) { } // test sparse mask set to scalar - rc = grb::clear( dst ); - if( rc == SUCCESS ) { - rc = grb::clear( src ); - } + rc = grb::clear( src ); if( rc == SUCCESS ) { rc = grb::setElement( src, 1.5, n / 2 ); } @@ -264,19 +263,24 @@ void grb_program( const size_t &n, grb::RC &rc ) { rc = grb::set( dst, src, 3.0 ); } if( rc != SUCCESS ) { - std::cerr << "\t Sparse-mask set to scalar FAILED with error code " << grb::toString( rc ) << "\n"; + std::cerr << "\t Sparse-mask set to scalar FAILED with error code " + << grb::toString( rc ) << "\n"; } else { if( nnz( dst ) != 1 ) { - std::cerr << "\t (sparse-mask-set-scalar) unexpected number of nonzeroes " << nnz( dst ) << ", expected 1.\n"; + std::cerr << "\t (sparse-mask-set-scalar) unexpected number of nonzeroes " + << nnz( dst ) << ", expected 1.\n"; rc = FAILED; } - for( const auto & pair : dst ) { + for( const auto &pair : dst ) { if( pair.first == n / 2 && pair.second != 3.0 ) { - std::cerr << "\t (sparse-mask-set-to-scalar) unexpected entry ( " << pair.first << ", " << pair.second << " ): expected value 3.0\n"; + std::cerr << "\t (sparse-mask-set-to-scalar) unexpected entry " + << "( " << pair.first << ", " << pair.second << " ), expected value 3.0\n"; rc = FAILED; } if( pair.first != n / 2 ) { - std::cerr << "\t (sparse-mask-set-to-scalar) unexpected entry ( " << pair.first << ", " << pair.second << " ): expected no entry at this position\n"; + std::cerr << "\t (sparse-mask-set-to-scalar) unexpected entry " + << "( " << pair.first << ", " << pair.second << " ), " + << "expected no entry at this position\n"; rc = FAILED; } } @@ -290,19 +294,25 @@ void grb_program( const size_t &n, grb::RC &rc ) { rc = rc ? rc : grb::setElement( src, 1.5, 0 ); rc = rc ? rc : grb::set( dst, src, 3.0 ); if( rc != SUCCESS ) { - std::cerr << "\t Sparse-mask set to scalar (re-entrant) FAILED with error code " << grb::toString( rc ) << "\n"; + std::cerr << "\t Sparse-mask set to scalar (re-entrant) FAILED with error code " + << grb::toString( rc ) << "\n"; } else { if( nnz( dst ) != 1 ) { - std::cerr << "\t (sparse-mask-set-scalar-reentrant) unexpected number of nonzeroes " << nnz( dst ) << ", expected 1.\n"; + std::cerr << "\t (sparse-mask-set-scalar-reentrant) unexpected number of " + << "nonzeroes " << nnz( dst ) << ", expected 1.\n"; rc = FAILED; } for( const auto &pair : dst ) { - if( (pair.first == 0 || pair.first == n/2) && pair.second != 3.0 ) { - std::cerr << "\t (sparse-mask-set-scalar-reentrant) unexpected entry ( " << pair.first << ", " << pair.second << " ): expected value 3.0\n"; + if( pair.first == 0 && pair.second != 3.0 ) { + std::cerr << "\t (sparse-mask-set-scalar-reentrant) unexpected entry " + << "( " << pair.first << ", " << pair.second << " ), " + << "expected value 3.0\n"; rc = FAILED; } - if( pair.first != 0 && pair.first != n/2 ) { - std::cerr << "\t (sparse-mask-set-scalar-reentrant) unexpected entry ( " << pair.first << ", " << pair.second << " ): expected no entry at this position\n"; + if( pair.first != 0 ) { + std::cerr << "\t (sparse-mask-set-scalar-reentrant) unexpected entry " + << "( " << pair.first << ", " << pair.second << " ), " + << "expected no entry at this position\n"; rc = FAILED; } } @@ -312,19 +322,14 @@ void grb_program( const size_t &n, grb::RC &rc ) { } // test sparse inverted mask set to empty - rc = grb::clear( dst ); - if( rc == SUCCESS ) { - rc = grb::set< grb::descriptors::invert_mask >( dst, src, src ); - } + rc = grb::set< grb::descriptors::invert_mask >( dst, src, src ); if( rc != SUCCESS ) { - std::cerr << "\t Sparse-inverted-mask set to empty FAILED with error " - "code " - << grb::toString( rc ) << "\n"; + std::cerr << "\t Sparse-inverted-mask set to empty FAILED with error code " + << grb::toString( rc ) << "\n"; } else { if( nnz( dst ) != 0 ) { - std::cerr << "\t (sparse-inverted-mask-set-empty) unexpected " - "number of nonzeroes " - << nnz( dst ) << ", expected 0.\n"; + std::cerr << "\t (sparse-inverted-mask-set-empty) unexpected number of " + << "nonzeroes " << nnz( dst ) << ", expected 0.\n"; rc = FAILED; } } @@ -334,10 +339,7 @@ void grb_program( const size_t &n, grb::RC &rc ) { // test sparse inverted mask set grb::Vector< bool > mask( n ); - rc = grb::clear( dst ); - if( rc == SUCCESS ) { - rc = grb::setElement( mask, true, n / 2 ); - } + rc = grb::setElement( mask, true, n / 2 ); if( rc == SUCCESS ) { rc = grb::set( src, 1.5 ); } @@ -345,25 +347,52 @@ void grb_program( const size_t &n, grb::RC &rc ) { rc = grb::set< grb::descriptors::invert_mask >( dst, mask, src ); } if( rc != SUCCESS ) { - std::cerr << "\t Sparse inverted-mask set FAILED with error code " << grb::toString( rc ) << "\n"; + std::cerr << "\t Sparse inverted-mask set FAILED with error code " + << grb::toString( rc ) << "\n"; } else { if( nnz( dst ) != n - 1 ) { - std::cerr << "\t (sparse-inverted-mask-set) unexpected number of " - "nonzeroes " - << nnz( dst ) << ", expected " << ( n - 1 ) << ".\n"; + std::cerr << "\t (sparse-inverted-mask-set) unexpected number of nonzeroes " + << nnz( dst ) << ", expected " << (n - 1) << ".\n"; rc = FAILED; } - for( const auto & pair : dst ) { + for( const auto &pair : dst ) { if( pair.first == n / 2 ) { - std::cerr << "\t (sparse-inverted-mask-set) unexpected entry ( " << pair.first << ", " << pair.second << " ): this position should have been empty\n"; + std::cerr << "\t (sparse-inverted-mask-set) unexpected entry " + << "( " << pair.first << ", " << pair.second << " ), " + << "this position should have been empty\n"; rc = FAILED; } else if( pair.second != 1.5 ) { - std::cerr << "\t (sparse-inverted-mask-set) unexpected entry ( " << pair.first << ", " << pair.second << " ): expected value 1.5.\n"; + std::cerr << "\t (sparse-inverted-mask-set) unexpected entry " + << "( " << pair.first << ", " << pair.second << " ), " + << "expected value 1.5.\n"; rc = FAILED; } } } - // if( rc != SUCCESS ) { return; } + if( rc != SUCCESS ) { + return; + } + + rc = grb::clear( src ); + if( rc == SUCCESS ) { + rc = grb::set( dst, src ); + } + if( rc != SUCCESS ) { + std::cerr << "\t Set to empty vector FAILED with error code " + << grb::toString( rc ) << "\n"; + } else { + if( nnz( dst ) != 0 ) { + std::cerr << "\t (set-to-empty) unexpected number of nonzeroes " + << nnz( dst ) << ", expected 0.\n"; + rc = FAILED; + } + for( const auto &pair : dst ) { + std::cerr << "\t (set-to-empty) unexpected entry " + << "( " << pair.first << ", " << pair.second << " ), " + << "this position should have been empty\n"; + rc = FAILED; + } + } // done return; @@ -397,8 +426,8 @@ int main( int argc, char ** argv ) { } if( printUsage ) { std::cerr << "Usage: " << argv[ 0 ] << " [n]\n"; - std::cerr << " -n (optional, default is 100): an even integer, the " - "test size.\n"; + std::cerr << " -n (optional, default is 100): an even integer, " + << "the test size.\n"; return 1; } @@ -416,3 +445,4 @@ int main( int argc, char ** argv ) { } return 0; } + From ac15c399bb847000b3a78d059218a89f16eaa951 Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Mon, 17 Oct 2022 17:40:00 +0200 Subject: [PATCH 11/14] Fix grb::set behaviour under the dense descriptor when use of masks yields sparse output --- include/graphblas/reference/io.hpp | 101 +++++++++++++++++++---------- 1 file changed, 67 insertions(+), 34 deletions(-) diff --git a/include/graphblas/reference/io.hpp b/include/graphblas/reference/io.hpp index 38230d135..e95b0b142 100644 --- a/include/graphblas/reference/io.hpp +++ b/include/graphblas/reference/io.hpp @@ -367,7 +367,8 @@ namespace grb { typename Coords > RC set( - Vector< DataType, reference, Coords > &x, const T val, + Vector< DataType, reference, Coords > &x, + const T val, const Phase &phase = EXECUTE, const typename std::enable_if< !grb::is_object< DataType >::value && @@ -381,6 +382,12 @@ namespace grb { "called with a value type that does not match that of the given vector" ); + // dynamic checks + const size_t n = size( x ); + if( (descr & descriptors::dense) && nnz( x ) < n ) { + return ILLEGAL; + } + if( phase == RESIZE ) { return SUCCESS; } @@ -390,9 +397,10 @@ namespace grb { const DataType toCopy = static_cast< DataType >( val ); // make vector dense if it was not already - internal::getCoordinates( x ).assignAll(); + if( !(descr & descriptors::dense) ) { + internal::getCoordinates( x ).assignAll(); + } DataType * const raw = internal::getRaw( x ); - const size_t n = internal::getCoordinates( x ).size(); #ifdef _H_GRB_REFERENCE_OMP_IO #pragma omp parallel @@ -460,35 +468,48 @@ namespace grb { "vector" ); - if( phase == RESIZE ) { - return SUCCESS; - } - assert( phase == EXECUTE ); - // catch empty mask if( size( m ) == 0 ) { - return set< descr >( x, val ); + return set< descr >( x, val, phase ); } // dynamic sanity checks - if( size( x ) != size( m ) ) { + const size_t sizex = size( x ); + if( sizex != size( m ) ) { return MISMATCH; } + if( (descr & descriptors::dense) && + (nnz( x ) < sizex || nnz( m ) < sizex) + ) { + return ILLEGAL; + } - // pre-cast value to be copied - const DataType toCopy = static_cast< DataType >( val ); - - // make vector dense if it was not already - DataType * const raw = internal::getRaw( x ); - auto & coors = internal::getCoordinates( x ); - const auto & m_coors = internal::getCoordinates( m ); - auto m_p = internal::getRaw( m ); + // handle trivial resize + if( phase == RESIZE ) { + return SUCCESS; + } + assert( phase == EXECUTE ); // make the vector empty unless the dense descriptor is provided - if( !( descr & descriptors::dense ) ) { + const bool mask_is_dense = (descr & descriptors::structural) && + !(descr & descriptors::invert_mask) && ( + (descr & descriptors::dense) || + nnz( m ) == sizex + ); + if( !((descr & descriptors::dense) && mask_is_dense) ) { internal::getCoordinates( x ).clear(); + } else if( mask_is_dense ) { + // dispatch to faster variant if mask is structurally dense + return set< descr >( x, val, phase ); } + // pre-cast value to be copied and get coordinate handles + const DataType toCopy = static_cast< DataType >( val ); + DataType * const raw = internal::getRaw( x ); + auto &coors = internal::getCoordinates( x ); + const auto &m_coors = internal::getCoordinates( m ); + const MaskType * const m_p = internal::getRaw( m ); + #ifdef _H_GRB_REFERENCE_OMP_IO #pragma omp parallel { @@ -520,18 +541,20 @@ namespace grb { } #ifdef _H_GRB_REFERENCE_OMP_IO if( !coors.asyncAssign( index, localUpdate ) ) { - (void)++asyncAssigns; + (void) ++asyncAssigns; } if( asyncAssigns == maxAsyncAssigns ) { - (void)coors.joinUpdate( localUpdate ); + (void) coors.joinUpdate( localUpdate ); asyncAssigns = 0; } #else - (void)coors.assign( index ); + (void) coors.assign( index ); #endif - raw[ index ] = - internal::ValueOrIndex< descr, DataType, DataType >::getFromScalar( - toCopy, index ); + raw[ index ] = internal::ValueOrIndex< + descr, DataType, DataType + >::getFromScalar( + toCopy, index + ); } #ifdef _H_GRB_REFERENCE_OMP_IO while( !coors.joinUpdate( localUpdate ) ) {} @@ -570,21 +593,26 @@ namespace grb { ) { // static sanity checks NO_CAST_ASSERT( ( !(descr & descriptors::no_casting) || - std::is_same< DataType, T >::value ), "grb::set (Vector, at index)", + std::is_same< DataType, T >::value ), + "grb::set (Vector, at index)", "called with a value type that does not match that of the given " - "vector" ); + "vector" + ); if( phase == RESIZE ) { return SUCCESS; } assert( phase == EXECUTE ); // dynamic sanity checks - if( i >= internal::getCoordinates( x ).size() ) { + if( i >= size( x ) ) { return MISMATCH; } + if( (descr & descriptors::dense) && nnz( x ) < size( x ) ) { + return ILLEGAL; + } // do set - (void)internal::getCoordinates( x ).assign( i ); + (void) internal::getCoordinates( x ).assign( i ); internal::getRaw( x )[ i ] = static_cast< DataType >( val ); #ifdef _DEBUG @@ -622,16 +650,16 @@ namespace grb { ) { // static sanity checks NO_CAST_ASSERT( ( !(descr & descriptors::no_casting) || - std::is_same< OutputType, InputType >::value ), + std::is_same< OutputType, InputType >::value ), "grb::copy (Vector)", "called with vector parameters whose element data types do not match" ); constexpr bool out_is_void = std::is_void< OutputType >::value; constexpr bool in_is_void = std::is_void< OutputType >::value; - static_assert( ! in_is_void || out_is_void, + static_assert( !in_is_void || out_is_void, "grb::set (reference, vector <- vector, masked): " "if input is void, then the output must be also" ); - static_assert( ! ( descr & descriptors::use_index ) || ! out_is_void, + static_assert( !(descr & descriptors::use_index) || !out_is_void, "grb::set (reference, vector <- vector, masked): " "use_index descriptor cannot be set if output vector is void" ); @@ -664,7 +692,7 @@ namespace grb { const InputType * __restrict__ const src = internal::getRaw( y ); // make the vector empty unless the dense descriptor is provided - if( !( descr & descriptors::dense ) ) { + if( !(descr & descriptors::dense) ) { internal::getCoordinates( x ).clear(); } @@ -824,7 +852,12 @@ namespace grb { auto &x_coors = internal::getCoordinates( x ); // make the vector empty unless the dense descriptor is provided - if( !( descr & descriptors::dense ) ) { + const bool mask_is_dense = (descr & descriptors::structural) && + !(descr & descriptors::invert_mask) && ( + (descr && descriptors::dense) || + nnz( mask ) == grb::size( mask ) + ); + if( !((descr & descriptors::dense) && mask_is_dense) ) { internal::getCoordinates( x ).clear(); } From c3bd3170a9d995e1c8ed4100039c3a66fd3cd6ad Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Mon, 17 Oct 2022 17:40:20 +0200 Subject: [PATCH 12/14] The dense descriptor should also apply to the output vector --- include/graphblas/descriptors.hpp | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/include/graphblas/descriptors.hpp b/include/graphblas/descriptors.hpp index 1fe3f9836..a2b992c85 100644 --- a/include/graphblas/descriptors.hpp +++ b/include/graphblas/descriptors.hpp @@ -117,11 +117,11 @@ namespace grb { static constexpr Descriptor structural_complement = structural | invert_mask; /** - * Indicates that all input vectors to an ALP/GraphBLAS primitive are - * structurally dense. + * Indicates that all input and output vectors to an ALP/GraphBLAS primitive + * are structurally dense. * - * If a user passes this descriptor but one or more vectors input to the call - * are \em not structurally dense, then #ILLEGAL shall be returned. + * If a user passes this descriptor but one or more vectors to the call are + * \em not structurally dense, then #ILLEGAL shall be returned. * * \warning All vectors includes any vectors that operate as masks. * Thus if the primitive is to operate with structurally sparse masks @@ -134,6 +134,10 @@ namespace grb { * passing this descriptor to such primitive indicates that also the * output vector is structurally dense. * + * \warning For out-of-place operations with vector output(s), passing this + * descriptor also demands that the output vectors are already + * dense. + * * \warning Vectors with explicit zeroes (under the semiring passed to the * related primitive) will be computed with explicitly. * @@ -141,6 +145,7 @@ namespace grb { * 1) less run-time overhead as code handling sparsity is disabled; * 2) smaller binary sizes as code handling structurally sparse vectors is * not emitted (unless required elsewhere). + * * The consistent use of this descriptor is hence strongly encouraged. */ static constexpr Descriptor dense = 16; From b7181a3fa06e1c679b5977b4830ce83d23397e24 Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Mon, 17 Oct 2022 18:49:03 +0200 Subject: [PATCH 13/14] Harden unit test for grb::set -- test double set-to-empty, and test most/all variants of grb::set in the presence of a dense descriptor --- tests/unit/set.cpp | 312 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 312 insertions(+) diff --git a/tests/unit/set.cpp b/tests/unit/set.cpp index 2ba74d4e1..d456cdd7b 100644 --- a/tests/unit/set.cpp +++ b/tests/unit/set.cpp @@ -23,6 +23,295 @@ using namespace grb; +static grb::RC dense_tests( + grb::Vector< double > &dst, + grb::Vector< double > &src +) { + assert( size( dst ) == size( src ) ); + grb::Vector< bool > full_mask( size( dst ) ), one_mask( size( dst ) ); + grb::RC ret = grb::set( full_mask, false ); + ret = ret ? ret : grb::setElement( one_mask, false, size( dst ) / 2 ); + ret = ret ? ret : grb::clear( src ); + ret = ret ? ret : grb::clear( dst ); + if( ret != SUCCESS ) { + std::cerr << "\t initalisation of dense tests FAILED\n"; + return ret; + } + + std::cerr << "\t dense subtest 1:"; + ret = grb::setElement< descriptors::dense >( src, 3.14, 0 ); + if( ret != ILLEGAL ) { + std::cerr << " expected ILLEGAL, got " << toString( ret ) << "\n"; + return FAILED; + } + if( nnz( dst ) != 0 ) { + std::cerr << " expected 0, got " << nnz( dst ) << "\n"; + return FAILED; + } + + std::cerr << "\b 2:"; + ret = grb::set< descriptors::dense >( dst, 1.0 ); + if( ret != ILLEGAL ) { + std::cerr << " expected ILLEGAL, got " << toString( ret ) << "\n"; + return FAILED; + } + if( nnz( dst ) != 0 ) { + std::cerr << " expected 0, got " << nnz( dst ) << "\n"; + return FAILED; + } + + std::cerr << "\b 3:"; + ret = grb::set< descriptors::dense >( dst, one_mask, 1.0 ); + if( ret != ILLEGAL ) { + std::cerr << " expected ILLEGAL, got " << toString( ret ) << "\n"; + return FAILED; + } + if( nnz( dst ) != 0 ) { + std::cerr << " expected 0, got " << nnz( dst ) << "\n"; + return FAILED; + } + + std::cerr << "\b 4:"; + ret = grb::set< descriptors::dense >( dst, full_mask, 1.0 ); + if( ret != ILLEGAL ) { + std::cerr << " expected ILLEGAL, got " << toString( ret ) << "\n"; + return FAILED; + } + if( nnz( dst ) != 0 ) { + std::cerr << " expected 0, got " << nnz( dst ) << "\n"; + return FAILED; + } + + std::cerr << "\b 5:"; + ret = grb::set< descriptors::dense >( dst, src ); + if( ret != ILLEGAL ) { + std::cerr << " expected ILLEGAL, got " << toString( ret ) << "\n"; + return FAILED; + } + if( nnz( dst ) != 0 ) { + std::cerr << " expected 0, got " << nnz( dst ) << "\n"; + return FAILED; + } + + std::cerr << "\b 6:"; + ret = grb::set< descriptors::dense >( dst, one_mask, src ); + if( ret != ILLEGAL ) { + std::cerr << " expected ILLEGAL, got " << toString( ret ) << "\n"; + return FAILED; + } + if( nnz( dst ) != 0 ) { + std::cerr << " expected 0, got " << nnz( dst ) << "\n"; + return FAILED; + } + + std::cerr << "\b 7:"; + ret = grb::set< descriptors::dense >( dst, full_mask, src ); + if( ret != ILLEGAL ) { + std::cerr << " expected ILLEGAL, got " << toString( ret ) << "\n"; + return FAILED; + } + if( nnz( dst ) != 0 ) { + std::cerr << " expected 0, got " << nnz( dst ) << "\n"; + return FAILED; + } + + std::cerr << "\b 8:"; + ret = grb::set( src, 3.14 ); + ret = ret ? ret : grb::set< descriptors::dense >( dst, src ); + if( ret != ILLEGAL ) { + std::cerr << " expected ILLEGAL, got " << toString( ret ) << "\n"; + return FAILED; + } + if( nnz( dst ) != 0 ) { + std::cerr << " expected 0, got " << nnz( dst ) << "\n"; + return FAILED; + } + + std::cerr << "\b 9:"; + ret = grb::set< descriptors::dense >( dst, one_mask, src ); + if( ret != ILLEGAL ) { + std::cerr << " expected ILLEGAL, got " << toString( ret ) << "\n"; + return FAILED; + } + if( nnz( dst ) != 0 ) { + std::cerr << " expected 0, got " << nnz( dst ) << "\n"; + return FAILED; + } + + std::cerr << "\b 10:"; + ret = grb::set< descriptors::dense >( dst, full_mask, src ); + if( ret != ILLEGAL ) { + std::cerr << " expected ILLEGAL, got " << toString( ret ) << "\n"; + return FAILED; + } + if( nnz( dst ) != 0 ) { + std::cerr << " expected 0, got " << nnz( dst ) << "\n"; + return FAILED; + } + + std::cerr << "\b 11:"; + ret = grb::set( dst, 0 ); + ret = ret ? ret : grb::set< descriptors::dense >( dst, 1.0 ); + if( ret != SUCCESS ) { + std::cerr << " expected SUCCESS, got " << toString( ret ) << "\n"; + return FAILED; + } + if( nnz( dst ) != size( dst ) ) { + std::cerr << " expected " << size( dst ) << ", got " << nnz( dst ) << "\n"; + ret = FAILED; + } + for( const auto &pair : dst ) { + if( pair.second != 1.0 ) { + std::cerr << "\t got ( " << pair.first << ", " << pair.second << " ), " + << "expected 1\n"; + ret = FAILED; + } + } + if( ret != SUCCESS ) { return ret; } + + std::cerr << "\b 12:"; + ret = grb::set( dst, 0 ); + ret = ret ? ret : grb::set< descriptors::dense >( dst, one_mask, 1.0 ); + if( ret != ILLEGAL ) { + std::cerr << " expected ILLEGAL, got " << toString( ret ) << "\n"; + return FAILED; + } + ret = SUCCESS; + if( nnz( dst ) != size( dst ) ) { + std::cerr << " expected " << size( dst ) << ", got " << nnz( dst ) << "\n"; + ret = FAILED; + } + for( const auto &pair : dst ) { + if( pair.second != 0.0 ) { + std::cerr << "\t got ( " << pair.first << ", " << pair.second << " ), " + << "expected 0\n"; + ret = FAILED; + } + } + if( ret != SUCCESS ) { return ret; } + + std::cerr << "\b 13:"; + ret = grb::set< descriptors::dense >( dst, full_mask, 1.0 ); + if( ret != SUCCESS ) { + std::cerr << " expected SUCCESS, got " << toString( ret ) << "\n"; + return FAILED; + } + if( nnz( dst ) != 0 ) { + std::cerr << " expected 0, got " << nnz( dst ) << "\n"; + ret = FAILED; + } + for( const auto &pair : dst ) { + std::cerr << "\t got ( " << pair.first << ", " << pair.second << " ), " + << "expected no entries\n"; + ret = FAILED; + } + if( ret != SUCCESS ) { return ret; } + + std::cerr << "\b 14:"; + ret = grb::set( dst, 0 ); + ret = ret ? ret : grb::set< descriptors::dense | descriptors::invert_mask >( + dst, full_mask, 1.0 + ); + if( ret != SUCCESS ) { + std::cerr << " expected SUCCESS, got " << toString( ret ) << "\n"; + return FAILED; + } + if( nnz( dst ) != size( dst ) ) { + std::cerr << " expected " << size( dst ) << ", got " << nnz( dst ) << "\n"; + ret = FAILED; + } + for( const auto &pair : dst ) { + if( pair.second != 1.0 ) { + std::cerr << "\t got ( " << pair.first << ", " << pair.second << " ), " + << "expected entry with value 1\n"; + ret = FAILED; + } + } + if( ret != SUCCESS ) { return ret; } + + std::cerr << "\b 15:"; + ret = grb::set< descriptors::dense >( dst, src ); + if( ret != SUCCESS ) { + std::cerr << " expected SUCCESS, got " << toString( ret ) << "\n"; + return FAILED; + } + if( nnz( dst ) != size( dst ) ) { + std::cerr << " expected " << size( dst ) << ", got " << nnz( dst ) << "\n"; + ret = FAILED; + } + for( const auto &pair : dst ) { + if( pair.second != 3.14 ) { + std::cerr << "\t got ( " << pair.first << ", " << pair.second << " ), " + << "expected 3.14\n"; + ret = FAILED; + } + } + if( ret != SUCCESS ) { return ret; } + + std::cerr << "\b 16:"; + ret = grb::set( dst, 0 ); + ret = ret ? ret : grb::set< descriptors::dense >( dst, one_mask, src ); + if( ret != ILLEGAL ) { + std::cerr << " expected ILLEGAL, got " << toString( ret ) << "\n"; + return FAILED; + } + ret = SUCCESS; + if( nnz( dst ) != size( dst ) ) { + std::cerr << " expected " << size( dst ) << ", got " << nnz( dst ) << "\n"; + ret = FAILED; + } + for( const auto &pair : dst ) { + if( pair.second != 0 ) { + std::cerr << "\t got ( " << pair.first << ", " << pair.second << " ), " + << "expected zero values\n"; + ret = FAILED; + } + } + if( ret != SUCCESS ) { return ret; } + + std::cerr << "\b 17:"; + ret = grb::set< descriptors::dense >( dst, full_mask, src ); + if( ret != SUCCESS ) { + std::cerr << " expected SUCCESS, got " << toString( ret ) << "\n"; + return FAILED; + } + if( nnz( dst ) != 0 ) { + std::cerr << " expected 0, got " << nnz( dst ) << "\n"; + ret = FAILED; + } + for( const auto &pair : dst ) { + std::cerr << "\t got ( " << pair.first << ", " << pair.second << " ), " + << "expected no entries\n"; + ret = FAILED; + } + if( ret != SUCCESS ) { return ret; } + + std::cerr << "\b 18:"; + ret = grb::set( dst, 0 ); + ret = ret ? ret : grb::set< descriptors::dense | descriptors::invert_mask >( + dst, full_mask, src + ); + if( ret != SUCCESS ) { + std::cerr << " expected SUCCESS, got " << toString( ret ) << "\n"; + return FAILED; + } + if( nnz( dst ) != size( dst ) ) { + std::cerr << " expected " << size( dst ) << ", got " << nnz( dst ) << "\n"; + ret = FAILED; + } + for( const auto &pair : dst ) { + if( pair.second != 3.14 ) { + std::cerr << "\t got ( " << pair.first << ", " << pair.second << " ), " + << "expected 3.14\n"; + ret = FAILED; + } + } + if( ret != SUCCESS ) { return ret; } + + std::cerr << "\b OK\n"; + return SUCCESS; +} + void grb_program( const size_t &n, grb::RC &rc ) { grb::Vector< double > dst( n ), src( n ); @@ -373,6 +662,7 @@ void grb_program( const size_t &n, grb::RC &rc ) { return; } + // test set-to-clear rc = grb::clear( src ); if( rc == SUCCESS ) { rc = grb::set( dst, src ); @@ -394,6 +684,28 @@ void grb_program( const size_t &n, grb::RC &rc ) { } } + // test double-set-to-clear + rc = grb::set( dst, src ); + if( rc != SUCCESS ) { + std::cerr << "\t Set to empty vector FAILED with error code " + << grb::toString( rc ) << "\n"; + } else { + if( nnz( dst ) != 0 ) { + std::cerr << "\t (set-to-empty) unexpected number of nonzeroes " + << nnz( dst ) << ", expected 0.\n"; + rc = FAILED; + } + for( const auto &pair : dst ) { + std::cerr << "\t (set-to-empty) unexpected entry " + << "( " << pair.first << ", " << pair.second << " ), " + << "this position should have been empty\n"; + rc = FAILED; + } + } + + // test behaviour under dense descriptor + rc = dense_tests( dst, src ); + // done return; } From 2b44d44da6c5f9af6aed1d2276285fa9f6e0b8d7 Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Mon, 17 Oct 2022 18:49:24 +0200 Subject: [PATCH 14/14] Enhanced unit test found two more bugs in two grb::set variants --- include/graphblas/reference/io.hpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/include/graphblas/reference/io.hpp b/include/graphblas/reference/io.hpp index e95b0b142..5fae54d5f 100644 --- a/include/graphblas/reference/io.hpp +++ b/include/graphblas/reference/io.hpp @@ -674,7 +674,7 @@ namespace grb { return ILLEGAL; } if( descr & descriptors::dense ) { - if( nnz( y ) < size( y ) ) { + if( nnz( y ) < size( y ) || nnz( x ) < size( x ) ) { return ILLEGAL; } } @@ -822,7 +822,10 @@ namespace grb { return ILLEGAL; } if( descr & descriptors::dense ) { - if( nnz( y ) < grb::size( y ) || nnz( mask ) < grb::size( mask ) ) { + if( nnz( x ) < grb::size( x ) || + nnz( y ) < grb::size( y ) || + nnz( mask ) < grb::size( mask ) + ) { return ILLEGAL; } }