From e64a1f9441069159f6402bd69027cfa7c94ceebf Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Wed, 19 Oct 2022 14:38:06 +0200 Subject: [PATCH 1/9] Move some dispatches earlier in each eWiseApply function. Add checks for ILLEGAL arguments to eWiseApply with a dense descriptor. Add one missing clear in one eWiseApply variant. --- include/graphblas/reference/blas1.hpp | 140 ++++++++++++++++++-------- 1 file changed, 100 insertions(+), 40 deletions(-) diff --git a/include/graphblas/reference/blas1.hpp b/include/graphblas/reference/blas1.hpp index 853add992..9e612731b 100644 --- a/include/graphblas/reference/blas1.hpp +++ b/include/graphblas/reference/blas1.hpp @@ -4008,6 +4008,10 @@ namespace grb { if( internal::getCoordinates( x ).size() != n ) { return MISMATCH; } + if( descr & descriptors::dense ) { + if( nnz( z ) < size( z ) ) { return ILLEGAL; } + if( nnz( x ) < size( x ) ) { return ILLEGAL; } + } if( phase == RESIZE ) { return SUCCESS; @@ -4067,6 +4071,9 @@ namespace grb { #ifdef _DEBUG std::cout << "In eWiseApply ([T1]<-T2<-T3), operator variant\n"; #endif + if( (descr & descriptors::dense) && nnz( z ) < size( z ) ) { + return ILLEGAL; + } if( phase == RESIZE ) { return SUCCESS; } @@ -4149,6 +4156,11 @@ namespace grb { if( internal::getCoordinates( mask ).size() != n ) { return MISMATCH; } + if( descr & descriptors::dense ) { + if( nnz( z ) < size( z ) ) { return ILLEGAL; } + if( nnz( x ) < size( x ) ) { return ILLEGAL; } + if( nnz( mask ) < size( mask ) ) { return ILLEGAL; } + } if( phase == RESIZE ) { return SUCCESS; @@ -4227,6 +4239,11 @@ namespace grb { if( internal::getCoordinates( y ).size() != n ) { return MISMATCH; } + if( descr & descriptors::dense ) { + if( nnz( z ) < size( z ) ) { return ILLEGAL; } + if( nnz( x ) < size( x ) ) { return ILLEGAL; } + if( nnz( y ) < size( y ) ) { return ILLEGAL; } + } if( phase == RESIZE ) { return SUCCESS; @@ -4294,6 +4311,10 @@ namespace grb { if( internal::getCoordinates( y ).size() != n ) { return MISMATCH; } + if( descr & descriptors::dense ) { + if( nnz( y ) < size( y ) ) { return ILLEGAL; } + if( nnz( z ) < size( z ) ) { return ILLEGAL; } + } if( phase == RESIZE ) { return SUCCESS; @@ -4334,7 +4355,8 @@ namespace grb { typename OutputType, typename InputType1, typename InputType2, typename Coords > - RC eWiseApply( Vector< OutputType, reference, Coords > &z, + RC eWiseApply( + Vector< OutputType, reference, Coords > &z, const Vector< InputType1, reference, Coords > &x, const InputType2 beta, const Monoid &monoid = Monoid(), @@ -4353,6 +4375,10 @@ namespace grb { if( internal::getCoordinates( x ).size() != n ) { return MISMATCH; } + if( (descr & descriptors::dense) ) { + if( nnz( z ) < size( z ) ) { return ILLEGAL; } + if( nnz( x ) < size( x ) ) { return ILLEGAL; } + } if( phase == RESIZE ) { return SUCCESS; @@ -4393,7 +4419,8 @@ namespace grb { typename InputType1, typename InputType2, typename Coords > - RC eWiseApply( Vector< OutputType, reference, Coords > &z, + RC eWiseApply( + Vector< OutputType, reference, Coords > &z, const Vector< MaskType, reference, Coords > &mask, const Vector< InputType1, reference, Coords > &x, const Vector< InputType2, reference, Coords > &y, @@ -4425,6 +4452,12 @@ namespace grb { if( internal::getCoordinates( mask ).size() != n ) { return MISMATCH; } + if( (descr & descriptors::dense) ) { + if( nnz( z ) < size( z ) ) { return ILLEGAL; } + if( nnz( x ) < size( x ) ) { return ILLEGAL; } + if( nnz( y ) < size( y ) ) { return ILLEGAL; } + if( nnz( mask ) < size( mask ) ) { return ILLEGAL; } + } if( phase == RESIZE ) { return SUCCESS; @@ -4533,6 +4566,11 @@ namespace grb { if( internal::getCoordinates( mask ).size() != n ) { return MISMATCH; } + if( descr & descriptors::dense ) { + if( nnz( z ) < size( z ) ) { return ILLEGAL; } + if( nnz( y ) < size( y ) ) { return ILLEGAL; } + if( nnz( mask ) < size( mask ) ) { return ILLEGAL; } + } if( phase == RESIZE ) { return SUCCESS; @@ -4607,6 +4645,11 @@ namespace grb { if( internal::getCoordinates( mask ).size() != n ) { return MISMATCH; } + if( descr & descriptors::dense ) { + if( nnz( z ) < size( z ) ) { return ILLEGAL; } + if( nnz( x ) < size( x ) ) { return ILLEGAL; } + if( nnz( mask ) < size( mask ) ) { return ILLEGAL; } + } if( phase == RESIZE ) { return SUCCESS; @@ -4723,24 +4766,26 @@ namespace grb { #ifdef _DEBUG std::cout << "In eWiseApply ([T1]<-T2<-[T3]), operator variant\n"; #endif - // sanity check + // check if we can dispatch + if( getID( z ) == getID( y ) ) { + return foldr< descr >( alpha, z, op ); + } + + // dynamic sanity checks const size_t n = internal::getCoordinates( z ).size(); if( internal::getCoordinates( y ).size() != n ) { return MISMATCH; } + if( descr & descriptors::dense ) { + if( nnz( z ) < size( z ) ) { return ILLEGAL; } + if( nnz( y ) < size( y ) ) { return ILLEGAL; } + } if( phase == RESIZE ) { return SUCCESS; } assert( phase == EXECUTE ); - // check if we can dispatch - if( static_cast< const void * >( &z ) == - static_cast< const void * >( &y ) - ) { - return foldr< descr >( alpha, z, op ); - } - // check for dense variant if( (descr & descriptors::dense) || internal::getCoordinates( y ).nonzeroes() == n @@ -4754,6 +4799,7 @@ namespace grb { } // we are in the sparse variant + internal::getCoordinates( z ).clear(); const bool * const null_mask = nullptr; const Coords * const null_coors = nullptr; return internal::sparse_apply_generic< false, false, true, false, descr >( @@ -4798,29 +4844,34 @@ namespace grb { return eWiseApply< descr >( z, alpha, y, op ); } - // sanity check - const size_t n = internal::getCoordinates( z ).size(); + // check delegate to unmasked + const size_t n = internal::getCoordinates( mask ).size(); + const auto &mask_coors = internal::getCoordinates( mask ); + if( (descr & descriptors::structural) && + !(descr & descriptors::invert_mask) && + mask_coors.nonzeroes() == n + ) { + return eWiseApply< descr >( z, alpha, y, op ); + } + + // sanity checks if( internal::getCoordinates( y ).size() != n ) { return MISMATCH; } - if( internal::getCoordinates( mask ).size() != n ) { + if( internal::getCoordinates( z ).size() != n ) { return MISMATCH; } + if( descr & descriptors::dense ) { + if( nnz( z ) < size( z ) ) { return ILLEGAL; } + if( nnz( y ) < size( y ) ) { return ILLEGAL; } + if( nnz( mask ) < size( mask ) ) { return ILLEGAL; } + } if( phase == RESIZE ) { return SUCCESS; } assert( phase == EXECUTE ); - // check delegate to unmasked - const auto &mask_coors = internal::getCoordinates( mask ); - if( (descr & descriptors::structural) && - !(descr & descriptors::invert_mask) && - mask_coors.nonzeroes() == n - ) { - return eWiseApply< descr >( z, alpha, y, op ); - } - auto &z_coors = internal::getCoordinates( z ); OutputType * const z_p = internal::getRaw( z ); const MaskType * const mask_p = internal::getRaw( mask ); @@ -4933,7 +4984,7 @@ namespace grb { #ifdef _DEBUG std::cout << "In eWiseApply ([T1]<-[T2]<-[T3]), operator variant\n"; #endif - // sanity check + // dynamic sanity checks auto &z_coors = internal::getCoordinates( z ); const size_t n = z_coors.size(); if( internal::getCoordinates( x ).size() != n || @@ -4944,17 +4995,20 @@ namespace grb { #endif return MISMATCH; } + if( descr & descriptors::dense ) { + if( nnz( z ) < size( z ) ) { return ILLEGAL; } + if( nnz( x ) < size( x ) ) { return ILLEGAL; } + if( nnz( y ) < size( y ) ) { return ILLEGAL; } + } - // check for possible shortcuts - if( static_cast< const void * >( &x ) == static_cast< const void * >( &y ) && - is_idempotent< OP >::value - ) { + // check for possible shortcuts, after dynamic checks + if( getID( x ) == getID( y ) && is_idempotent< OP >::value ) { return set< descr >( z, x, phase ); } - if( static_cast< const void * >( &x ) == static_cast< void * >( &z ) ) { + if( getID( x ) == getID( z ) ) { return foldl< descr >( z, y, op, phase ); } - if( static_cast< const void * >( &y ) == static_cast< void * >( &z ) ) { + if( getID( y ) == getID( z ) ) { return foldr< descr >( x, z, op, phase ); } @@ -5048,19 +5102,34 @@ namespace grb { return eWiseApply< descr >( z, x, y, op, phase ); } + // check if can delegate to unmasked variant + const auto &m_coors = internal::getCoordinates( mask ); + const size_t n = m_coors.size(); + if( m_coors.nonzeroes() == n && + (descr & descriptors::structural) && + !(descr & descriptors::invert_mask) + ) { + return eWiseApply< descr >( z, x, y, op ); + } + // other run-time checks auto &z_coors = internal::getCoordinates( z ); const auto &mask_coors = internal::getCoordinates( mask ); - const size_t n = z_coors.size(); if( internal::getCoordinates( x ).size() != n ) { return MISMATCH; } if( internal::getCoordinates( y ).size() != n ) { return MISMATCH; } - if( mask_coors.size() != n ) { + if( z_coors.size() != n ) { return MISMATCH; } + if( descr & descriptors::dense ) { + if( nnz( z ) < size( z ) ) { return ILLEGAL; } + if( nnz( x ) < size( x ) ) { return ILLEGAL; } + if( nnz( y ) < size( y ) ) { return ILLEGAL; } + if( nnz( mask ) < size( mask ) ) { return ILLEGAL; } + } if( phase == RESIZE ) { return SUCCESS; @@ -5073,18 +5142,9 @@ namespace grb { const InputType2 * const y_p = internal::getRaw( y ); const auto &x_coors = internal::getCoordinates( x ); const auto &y_coors = internal::getCoordinates( y ); - const auto &m_coors = internal::getCoordinates( mask ); const size_t sparse_loop = std::min( x_coors.nonzeroes(), y_coors.nonzeroes() ); - // check if can delegate to unmasked variant - if( m_coors.nonzeroes() == n && - (descr & descriptors::structural) && - !(descr & descriptors::invert_mask) - ) { - return eWiseApply< descr >( z, x, y, op ); - } - // the output sparsity structure is unknown a priori z_coors.clear(); From 92d22a372cd0d14ec57bf7350310711eaea03c72 Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Wed, 19 Oct 2022 14:48:14 +0200 Subject: [PATCH 2/9] Minor code fixes and one assert to masked_apply_generic to harden the code --- include/graphblas/reference/blas1.hpp | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/include/graphblas/reference/blas1.hpp b/include/graphblas/reference/blas1.hpp index 9e612731b..c89c51f5a 100644 --- a/include/graphblas/reference/blas1.hpp +++ b/include/graphblas/reference/blas1.hpp @@ -3242,8 +3242,9 @@ namespace grb { #ifdef _H_GRB_REFERENCE_OMP_BLAS1 if( asyncAssigns > maxAsyncAssigns - block_size ) { #ifdef _DEBUG - std::cout << "\t\t " << omp_get_thread_num() << ": clearing local update at block " - << b << ". It locally holds " << asyncAssigns << " entries. " + std::cout << "\t\t " << omp_get_thread_num() << ": " + << "clearing local update at block " << b << ". " + << "It locally holds " << asyncAssigns << " entries. " << "Update is at " << ( (void *)update ) << "\n"; #endif #ifndef NDEBUG @@ -3514,13 +3515,18 @@ namespace grb { return SUCCESS; } + /** + * \internal Whenever this function is called, the z_coors is assumed to be + * cleared. + */ template< bool left_scalar, bool right_scalar, bool left_sparse, bool right_sparse, Descriptor descr, class OP, typename OutputType, typename MaskType, typename InputType1, typename InputType2 > - RC masked_apply_generic( OutputType * const z_p, + RC masked_apply_generic( + OutputType * const z_p, Coordinates< reference > &z_coors, const MaskType * const mask_p, const Coordinates< reference > &mask_coors, @@ -3549,6 +3555,7 @@ namespace grb { assert( !left_sparse || left_identity != nullptr ); assert( !right_sparse || right_coors != nullptr ); assert( !right_sparse || right_identity != nullptr ); + assert( z_coors.nonzeroes() == 0 ); #ifdef _DEBUG std::cout << "\tinternal::masked_apply_generic called with nnz(mask)=" @@ -3886,15 +3893,13 @@ namespace grb { #endif } const InputType1 * const x_e = left_scalar ? - x_p : - ( + x_p : ( (!left_sparse || left_coors->assigned( i )) ? x_p + i : left_identity - ); + ); const InputType2 * const y_e = right_scalar ? - y_p : - ( + y_p : ( (!right_sparse || right_coors->assigned( i )) ? y_p + i : right_identity From b4fed981bfbf921c8f8b2f7ef006f8e152c32626 Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Wed, 19 Oct 2022 14:55:28 +0200 Subject: [PATCH 3/9] Bugfix: dense descriptor no longer applies to output in masked_apply_generic --- include/graphblas/reference/blas1.hpp | 63 +++++++++++---------------- 1 file changed, 26 insertions(+), 37 deletions(-) diff --git a/include/graphblas/reference/blas1.hpp b/include/graphblas/reference/blas1.hpp index c89c51f5a..3f77ffc05 100644 --- a/include/graphblas/reference/blas1.hpp +++ b/include/graphblas/reference/blas1.hpp @@ -3590,9 +3590,6 @@ namespace grb { size_t_block_size : op_block_size; - // whether we have a dense hint - constexpr bool dense = descr & descriptors::dense; - if( bigLoop ) { #ifdef _DEBUG std::cerr << "\t in bigLoop variant\n"; @@ -3684,15 +3681,13 @@ namespace grb { const size_t index = i + k; assert( index < n ); if( mask_b[ k ] ) { - if( !dense ) { #ifdef _H_GRB_REFERENCE_OMP_BLAS1 - if( !z_coors.asyncAssign( index, update ) ) { - (void) ++asyncAssigns; - } + if( !z_coors.asyncAssign( index, update ) ) { + (void) ++asyncAssigns; + } #else - (void) z_coors.assign( index ); + (void) z_coors.assign( index ); #endif - } *( z_p + index ) = z_b[ k ]; } } @@ -3710,19 +3705,17 @@ namespace grb { // scalar coda for( size_t i = end * block_size; i < n; ++i ) { if( mask_coors.template mask< descr >( i, mask_p ) ) { - if( !dense ) { #ifdef _H_GRB_REFERENCE_OMP_BLAS1 - if( !z_coors.asyncAssign( i, update ) ) { - (void) ++asyncAssigns; - } - if( asyncAssigns == maxAsyncAssigns ) { - (void) z_coors.joinUpdate( update ); - asyncAssigns = 0; - } + if( !z_coors.asyncAssign( i, update ) ) { + (void) ++asyncAssigns; + } + if( asyncAssigns == maxAsyncAssigns ) { + (void) z_coors.joinUpdate( update ); + asyncAssigns = 0; + } #else - (void) z_coors.assign( i ); + (void) z_coors.assign( i ); #endif - } const InputType1 * const x_e = left_scalar ? x_p : ( @@ -3840,19 +3833,17 @@ namespace grb { } for( size_t t = 0; t < block_size; ++t ) { if( mask_b[ t ] ) { - if( !dense ) { #ifndef _H_GRB_REFERENCE_OMP_BLAS1 - (void) z_coors.assign( indices[ t ] ); + (void) z_coors.assign( indices[ t ] ); #else - if( !z_coors.asyncAssign( indices[ t ], update ) ) { - (void) ++asyncAssigns; + if( !z_coors.asyncAssign( indices[ t ], update ) ) { + (void) ++asyncAssigns; #ifdef _DEBUG - std::cout << "\t\t now made " << asyncAssigns << " calls to asyncAssign; " - << "added index " << indices[ t ] << "\n"; -#endif - } + std::cout << "\t\t now made " << asyncAssigns << " calls to asyncAssign; " + << "added index " << indices[ t ] << "\n"; #endif } +#endif GRB_UTIL_IGNORE_MAYBE_UNINITIALIZED // z_b is computed from x_b and *( z_p + indices[ t ] ) = z_b[ t ]; // y_b, which are both initialised GRB_UTIL_RESTORE_WARNINGS // if mask_b is true @@ -3879,19 +3870,17 @@ namespace grb { continue; } } - if( !dense ) { #ifndef _H_GRB_REFERENCE_OMP_BLAS1 - (void) z_coors.assign( i ); + (void) z_coors.assign( i ); #else - if( !z_coors.asyncAssign( i, update ) ) { - (void) ++asyncAssigns; - } - if( asyncAssigns == maxAsyncAssigns ) { - (void) z_coors.joinUpdate( update ); - asyncAssigns = 0; - } -#endif + if( !z_coors.asyncAssign( i, update ) ) { + (void) ++asyncAssigns; } + if( asyncAssigns == maxAsyncAssigns ) { + (void) z_coors.joinUpdate( update ); + asyncAssigns = 0; + } +#endif const InputType1 * const x_e = left_scalar ? x_p : ( (!left_sparse || left_coors->assigned( i )) ? From c3fda7f05ee5cc496dd90ce1597a3eeb75a674fb Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Wed, 19 Oct 2022 17:46:06 +0200 Subject: [PATCH 4/9] BSP1D backend did not handle dense descriptor properly --- include/graphblas/bsp1d/blas1.hpp | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/include/graphblas/bsp1d/blas1.hpp b/include/graphblas/bsp1d/blas1.hpp index ac7bb3514..1feb02350 100644 --- a/include/graphblas/bsp1d/blas1.hpp +++ b/include/graphblas/bsp1d/blas1.hpp @@ -1477,7 +1477,9 @@ namespace grb { // check if can delegate to dense variant const size_t n = size( z ); - if( (descr & descriptors::dense) || nnz( x ) == n ) { + if( (descr & descriptors::dense) || ( + nnz( x ) == n && nnz( z ) == n + ) ) { return eWiseApply< descr | descriptors::dense >( z, x, beta, monoid.getOperator(), phase ); @@ -1488,7 +1490,7 @@ namespace grb { return MISMATCH; } if( descr & descriptors::dense ) { - if( nnz( x ) < n ) { + if( nnz( x ) < n || nnz( z ) < n ) { return ILLEGAL; } } @@ -1566,7 +1568,9 @@ namespace grb { // check if can delegate to dense variant const size_t n = size( z ); - if( (descr & descriptors::dense) || nnz( y ) == n ) { + if( (descr & descriptors::dense) || ( + nnz( y ) == n && nnz( z ) == n + ) ) { return eWiseApply< descr | descriptors::dense >( z, alpha, y, monoid.getOperator(), phase ); @@ -1577,7 +1581,7 @@ namespace grb { return MISMATCH; } if( descr & descriptors::dense ) { - if( nnz( y ) < n ) { + if( nnz( y ) < n || nnz( z ) < n ) { return ILLEGAL; } } @@ -1657,7 +1661,9 @@ namespace grb { // check if we can delegate to dense variant const size_t n = size( z ); - if( (descr & descriptors::dense) || (nnz( x ) == n && nnz( y ) == n) ) { + if( (descr & descriptors::dense) || ( + nnz( x ) == n && nnz( y ) == n && nnz( z ) == n + ) ) { return eWiseApply< descr | descriptors::dense >( z, x, y, monoid.getOperator(), phase ); @@ -1773,7 +1779,7 @@ namespace grb { return MISMATCH; } if( descr & descriptors::dense ) { - if( nnz( y ) < n || nnz( mask ) < n ) { + if( nnz( y ) < n || nnz( mask ) < n || nnz( z ) < n ) { return ILLEGAL; } } @@ -1879,6 +1885,9 @@ namespace grb { if( nnz( x ) < n ) { return ILLEGAL; } + if( nnz ( z ) < n ) { + return ILLEGAL; + } } // handle trivial resize phase @@ -1979,7 +1988,7 @@ namespace grb { return MISMATCH; } if( descr & descriptors::dense ) { - if( nnz( x ) < n || nnz( y ) < n ) { + if( nnz( x ) < n || nnz( y ) < n || nnz( z ) < n ) { return ILLEGAL; } if( nnz( mask ) < n ) { @@ -2499,7 +2508,7 @@ namespace grb { return MISMATCH; } if( descr & descriptors::dense ) { - if( nnz( x ) < n || nnz( y ) < n ) { + if( nnz( x ) < n || nnz( y ) < n || nnz( z ) < n ) { return ILLEGAL; } } @@ -2563,7 +2572,7 @@ namespace grb { return MISMATCH; } if( descr & descriptors::dense ) { - if( nnz( y ) < n ) { + if( nnz( y ) < n && nnz( z ) < n ) { return ILLEGAL; } } @@ -2624,7 +2633,7 @@ namespace grb { return MISMATCH; } if( descr & descriptors::dense ) { - if( nnz( x ) < n ) { + if( nnz( x ) < n && nnz( z ) < n ) { return ILLEGAL; } } From bfdbb22fead24ff8ad7bfeef491381296b626b75 Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Wed, 19 Oct 2022 17:46:34 +0200 Subject: [PATCH 5/9] Minor code style change and make it easier to trace bugs (by adding asserts on return codes) --- tests/unit/ewiseapply.cpp | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/tests/unit/ewiseapply.cpp b/tests/unit/ewiseapply.cpp index fccc565b5..20fe590a1 100644 --- a/tests/unit/ewiseapply.cpp +++ b/tests/unit/ewiseapply.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 > out( n ), left( n ), right( n ); grb::Vector< bool > mask( n ); grb::Vector< size_t > temp( n ); @@ -59,6 +59,7 @@ void grb_program( const size_t & n, grb::RC & rc ) { // test operator versions first, dense vectors only, without masks rc = grb::eWiseApply( out, left, 0.25, plusM.getOperator() ); + assert( rc == SUCCESS ); if( rc == SUCCESS ) { if( nnz( out ) != size( out ) ) { std::cerr << "\tunexpected number of nonzeroes ( " << nnz( out ) << " ), " @@ -81,6 +82,7 @@ void grb_program( const size_t & n, grb::RC & rc ) { } rc = grb::eWiseApply( out, 0.25, left, plusM.getOperator() ); + assert( rc == SUCCESS ); if( rc == SUCCESS ) { if( nnz( out ) != size( out ) ) { std::cerr << "\tunexpected number of nonzeroes ( " << nnz( out ) << ", " @@ -102,6 +104,7 @@ void grb_program( const size_t & n, grb::RC & rc ) { } rc = grb::eWiseApply( out, left, left, plusM.getOperator() ); + assert( rc == SUCCESS ); if( rc == SUCCESS ) { if( nnz( out ) != size( out ) ) { std::cerr << "\tunexpected number of nonzeroes ( " << nnz( out ) << ", " @@ -124,6 +127,7 @@ void grb_program( const size_t & n, grb::RC & rc ) { // operator versions, dense vectors only, with masks rc = grb::eWiseApply( out, mask, left, 0.25, plusM.getOperator() ); + assert( rc == SUCCESS ); if( rc == SUCCESS ) { if( grb::nnz( out ) != grb::nnz( mask ) ) { std::cerr << "\tunexpected number of nonzeroes ( " << grb::nnz( out ) @@ -150,7 +154,9 @@ void grb_program( const size_t & n, grb::RC & rc ) { } else { return; } + rc = grb::eWiseApply( out, mask, 0.25, left, plusM.getOperator() ); + assert( rc == SUCCESS ); if( rc == SUCCESS ) { if( grb::nnz( out ) != grb::nnz( mask ) ) { std::cerr << "\tunexpected number of nonzeroes ( " << grb::nnz( out ) @@ -176,7 +182,9 @@ void grb_program( const size_t & n, grb::RC & rc ) { } else { return; } + rc = grb::eWiseApply( out, mask, left, left, plusM.getOperator() ); + assert( rc == SUCCESS ); if( rc == SUCCESS ) { if( grb::nnz( out ) != grb::nnz( mask ) ) { std::cerr << "\tunexpected number of nonzeroes ( " << grb::nnz( out ) @@ -205,6 +213,7 @@ void grb_program( const size_t & n, grb::RC & rc ) { // monoid version, dense vectors, unmasked rc = grb::eWiseApply( out, left, 0.25, plusM ); + assert( rc == SUCCESS ); if( rc == SUCCESS ) { if( nnz( out ) != size( out ) ) { std::cerr << "\tunexpected number of nonzeroes ( " << nnz( out ) << ", " @@ -226,6 +235,7 @@ void grb_program( const size_t & n, grb::RC & rc ) { } rc = grb::eWiseApply( out, 0.25, left, plusM ); + assert( rc == SUCCESS ); if( rc == SUCCESS ) { if( nnz( out ) != size( out ) ) { std::cerr << "\tunexpected number of nonzeroes ( " << nnz( out ) << ", " @@ -247,6 +257,7 @@ void grb_program( const size_t & n, grb::RC & rc ) { } rc = grb::eWiseApply( out, left, left, plusM.getOperator() ); + assert( rc == SUCCESS ); if( rc == SUCCESS ) { if( nnz( out ) != size( out ) ) { std::cerr << "\tunexpected number of nonzeroes ( " << nnz( out ) @@ -269,6 +280,7 @@ void grb_program( const size_t & n, grb::RC & rc ) { // monoid versions, dense vectors, with masks rc = grb::eWiseApply( out, mask, left, 0.25, plusM ); + assert( rc == SUCCESS ); if( rc == SUCCESS ) { if( grb::nnz( out ) != grb::nnz( mask ) ) { std::cerr << "\tunexpected number of nonzeroes ( " << grb::nnz( out ) @@ -294,7 +306,9 @@ void grb_program( const size_t & n, grb::RC & rc ) { } else { return; } + rc = grb::eWiseApply( out, mask, 0.25, left, plusM ); + assert( rc == SUCCESS ); if( rc == SUCCESS ) { if( grb::nnz( out ) != grb::nnz( mask ) ) { std::cerr << "\tunexpected number of nonzeroes ( " << grb::nnz( out ) @@ -320,7 +334,9 @@ void grb_program( const size_t & n, grb::RC & rc ) { } else { return; } + rc = grb::eWiseApply( out, mask, left, left, plusM ); + assert( rc == SUCCESS ); if( rc == SUCCESS ) { if( grb::nnz( out ) != grb::nnz( mask ) ) { std::cerr << "\tunexpected number of nonzeroes ( " << grb::nnz( out ) @@ -349,6 +365,7 @@ void grb_program( const size_t & n, grb::RC & rc ) { // monoid version, sparse vectors, unmasked rc = grb::eWiseApply( out, right, 0.25, plusM ); + assert( rc == SUCCESS ); if( rc == SUCCESS ) { if( nnz( out ) != size( out ) ) { std::cerr << "\tunexpected number of nonzeroes ( " << nnz( out ) << ", " @@ -378,6 +395,7 @@ void grb_program( const size_t & n, grb::RC & rc ) { } rc = grb::eWiseApply( out, 0.25, right, plusM ); + assert( rc == SUCCESS ); if( rc == SUCCESS ) { if( nnz( out ) != size( out ) ) { std::cerr << "\tunexpected number of nonzeroes ( " << nnz( out ) << " ), " @@ -407,6 +425,7 @@ void grb_program( const size_t & n, grb::RC & rc ) { } rc = grb::eWiseApply( out, left, right, plusM ); + assert( rc == SUCCESS ); if( rc == SUCCESS ) { if( nnz( out ) != size( out ) ) { std::cerr << "\tunexpected number of nonzeroes ( " << nnz( out ) << " ), " @@ -436,6 +455,7 @@ void grb_program( const size_t & n, grb::RC & rc ) { } rc = grb::eWiseApply( out, right, left, plusM ); + assert( rc == SUCCESS ); if( rc == SUCCESS ) { if( nnz( out ) != size( right ) ) { std::cerr << "\tunexpected number of nonzeroes ( " << nnz( out ) << ", " @@ -465,6 +485,7 @@ void grb_program( const size_t & n, grb::RC & rc ) { } rc = grb::eWiseApply( out, right, right, plusM ); + assert( rc == SUCCESS ); if( rc == SUCCESS ) { if( nnz( out ) != nnz( right ) ) { std::cerr << "\tunexpected number of nonzeroes ( " << nnz( out ) << ", " @@ -493,6 +514,7 @@ void grb_program( const size_t & n, grb::RC & rc ) { // monoid version, sparse vectors, with masks rc = grb::eWiseApply( out, mask, right, 0.25, plusM ); + assert( rc == SUCCESS ); if( rc == SUCCESS ) { if( nnz( out ) != size( out ) / 2 ) { std::cerr << "\tunexpected number of nonzeroes ( " << nnz( out ) << ", " @@ -528,6 +550,7 @@ void grb_program( const size_t & n, grb::RC & rc ) { } rc = grb::eWiseApply( out, mask, 0.25, right, plusM ); + assert( rc == SUCCESS ); if( rc == SUCCESS ) { if( nnz( out ) != size( out ) / 2 ) { std::cerr << "\tunexpected number of nonzeroes ( " << nnz( out ) << ", " @@ -563,6 +586,7 @@ void grb_program( const size_t & n, grb::RC & rc ) { } rc = grb::eWiseApply( out, mask, left, right, plusM ); + assert( rc == SUCCESS ); if( rc == SUCCESS ) { if( nnz( out ) != size( out ) / 2 ) { std::cerr << "\tunexpected number of nonzeroes ( " << nnz( out ) << ", " @@ -598,6 +622,7 @@ void grb_program( const size_t & n, grb::RC & rc ) { } rc = grb::eWiseApply( out, mask, right, left, plusM ); + assert( rc == SUCCESS ); if( rc == SUCCESS ) { if( nnz( out ) != size( right ) / 2 ) { std::cerr << "\tunexpected number of nonzeroes ( " << nnz( out ) << ", " @@ -633,6 +658,7 @@ void grb_program( const size_t & n, grb::RC & rc ) { } rc = grb::eWiseApply( out, mask, right, right, plusM ); + assert( rc == SUCCESS ); if( rc == SUCCESS ) { if( nnz( out ) != nnz( right ) / 2 ) { std::cerr << "\tunexpected number of nonzeroes ( " << nnz( out ) << ", " From e98ebe2c0f7449f541e022af465eeb8c56073d31 Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Wed, 19 Oct 2022 18:03:25 +0200 Subject: [PATCH 6/9] Subissue #3 in issue #557 --- include/graphblas/reference/blas1.hpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/include/graphblas/reference/blas1.hpp b/include/graphblas/reference/blas1.hpp index 3f77ffc05..522098692 100644 --- a/include/graphblas/reference/blas1.hpp +++ b/include/graphblas/reference/blas1.hpp @@ -3705,6 +3705,11 @@ namespace grb { // scalar coda for( size_t i = end * block_size; i < n; ++i ) { if( mask_coors.template mask< descr >( i, mask_p ) ) { + if( left_sparse && right_sparse ) { + if( !left_coors->assigned( i ) && !right_coors->assigned( i ) ) { + continue; + } + } #ifdef _H_GRB_REFERENCE_OMP_BLAS1 if( !z_coors.asyncAssign( i, update ) ) { (void) ++asyncAssigns; From 886f045fd9a21bed1ef8ae4187fcf85ed5b201e8 Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Wed, 19 Oct 2022 18:13:41 +0200 Subject: [PATCH 7/9] This should resolve subissue 4 of issue #557 --- include/graphblas/reference/blas1.hpp | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/include/graphblas/reference/blas1.hpp b/include/graphblas/reference/blas1.hpp index 522098692..7b72c0dd1 100644 --- a/include/graphblas/reference/blas1.hpp +++ b/include/graphblas/reference/blas1.hpp @@ -3214,17 +3214,19 @@ namespace grb { // part that may or may not be vectorised (can we do something about this??) for( size_t i = 0; i < block_size; ++i ) { if( !masked || mask[ i ] ) { + if( y_m[ i ] || monoid ) { #ifndef _H_GRB_REFERENCE_OMP_BLAS1 - (void) z_coors.assign( offsets[ i ] ); + (void) z_coors.assign( offsets[ i ] ); #else - if( !z_coors.asyncAssign( offsets[ i ], update ) ) { - (void) ++asyncAssigns; + if( !z_coors.asyncAssign( offsets[ i ], update ) ) { + (void) ++asyncAssigns; #ifdef _DEBUG - std::cout << "\t\t now made " << asyncAssigns - << " calls to asyncAssign; " << "added index " << offsets[ i ] << "\n"; + std::cout << "\t\t now made " << asyncAssigns << " calls to " + << "asyncAssign; " << "added index " << offsets[ i ] << "\n"; #endif - } + } #endif + } } } // perform scatter From fb852b17294bd0d8848b920d754eb0dddd802a65 Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Wed, 19 Oct 2022 19:22:04 +0200 Subject: [PATCH 8/9] Check for trivial dispatch before dispatching based on IDs --- include/graphblas/reference/blas1.hpp | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/include/graphblas/reference/blas1.hpp b/include/graphblas/reference/blas1.hpp index 7b72c0dd1..6304a9007 100644 --- a/include/graphblas/reference/blas1.hpp +++ b/include/graphblas/reference/blas1.hpp @@ -4767,11 +4767,6 @@ namespace grb { #ifdef _DEBUG std::cout << "In eWiseApply ([T1]<-T2<-[T3]), operator variant\n"; #endif - // check if we can dispatch - if( getID( z ) == getID( y ) ) { - return foldr< descr >( alpha, z, op ); - } - // dynamic sanity checks const size_t n = internal::getCoordinates( z ).size(); if( internal::getCoordinates( y ).size() != n ) { @@ -4782,6 +4777,16 @@ namespace grb { if( nnz( y ) < size( y ) ) { return ILLEGAL; } } + // check for trivial op + if( n == 0 ) { + return SUCCESS; + } + + // check if we can dispatch + if( getID( z ) == getID( y ) ) { + return foldr< descr >( alpha, z, op ); + } + if( phase == RESIZE ) { return SUCCESS; } @@ -5002,6 +5007,11 @@ namespace grb { if( nnz( y ) < size( y ) ) { return ILLEGAL; } } + // trivial dispatch + if( n == 0 ) { + return SUCCESS; + } + // check for possible shortcuts, after dynamic checks if( getID( x ) == getID( y ) && is_idempotent< OP >::value ) { return set< descr >( z, x, phase ); From de6f92720dff536c55f50f7acfd66192cefa7ab2 Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Fri, 21 Oct 2022 11:26:35 +0200 Subject: [PATCH 9/9] Code review: found two issues, herewith fixed --- include/graphblas/bsp1d/blas1.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/graphblas/bsp1d/blas1.hpp b/include/graphblas/bsp1d/blas1.hpp index 1feb02350..579d02940 100644 --- a/include/graphblas/bsp1d/blas1.hpp +++ b/include/graphblas/bsp1d/blas1.hpp @@ -2572,7 +2572,7 @@ namespace grb { return MISMATCH; } if( descr & descriptors::dense ) { - if( nnz( y ) < n && nnz( z ) < n ) { + if( nnz( y ) < n || nnz( z ) < n ) { return ILLEGAL; } } @@ -2633,7 +2633,7 @@ namespace grb { return MISMATCH; } if( descr & descriptors::dense ) { - if( nnz( x ) < n && nnz( z ) < n ) { + if( nnz( x ) < n || nnz( z ) < n ) { return ILLEGAL; } }