From 4cc34df1ae3ef3e7c7e3aca809a38acc70146327 Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Tue, 25 Oct 2022 13:14:57 +0200 Subject: [PATCH 1/5] Fixes the found bug --- include/graphblas/reference/blas1.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/graphblas/reference/blas1.hpp b/include/graphblas/reference/blas1.hpp index ace714eeb..bd8856167 100644 --- a/include/graphblas/reference/blas1.hpp +++ b/include/graphblas/reference/blas1.hpp @@ -3306,7 +3306,7 @@ namespace grb { #endif for( ; k < loop_coors.nonzeroes(); ++k ) { const size_t index = loop_coors.index( k ); - if( masked && mask_coors->template mask< descr >( index, mask_p ) ) { + if( masked && !mask_coors->template mask< descr >( index, mask_p ) ) { continue; } RC rc = SUCCESS; @@ -3518,7 +3518,7 @@ namespace grb { if( loop_coors.assigned( index ) ) { continue; } - if( masked && mask_coors->template mask< descr >( index, mask_p ) ) { + if( masked && !mask_coors->template mask< descr >( index, mask_p ) ) { continue; } #ifndef _H_GRB_REFERENCE_OMP_BLAS1 From 45936022ef6785b27f05314215f7917623ab2027 Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Tue, 25 Oct 2022 13:15:30 +0200 Subject: [PATCH 2/5] Automatically start the eWiseApply test for smaller sizes also --- tests/unit/unittests.sh | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/tests/unit/unittests.sh b/tests/unit/unittests.sh index b4cc5afbc..1de016563 100755 --- a/tests/unit/unittests.sh +++ b/tests/unit/unittests.sh @@ -194,6 +194,13 @@ for MODE in debug ndebug; do grep 'Test OK' ${TEST_OUT_DIR}/pinnedVector_${MODE}_${BACKEND}_${P}_${T}.log || echo "Test FAILED" echo " " + echo ">>> [x] [ ] Testing grb::eWiseApply using (+,0) on vectors" + echo " of doubles of size 14." + $runner ${TEST_BIN_DIR}/ewiseapply_${MODE}_${BACKEND} 14 &> ${TEST_OUT_DIR}/ewiseapply_small_${MODE}_${BACKEND}_${P}_${T} + head -1 ${TEST_OUT_DIR}/ewiseapply_small_${MODE}_${BACKEND}_${P}_${T} + grep 'Test OK' ${TEST_OUT_DIR}/ewiseapply_small_${MODE}_${BACKEND}_${P}_${T} || echo "Test FAILED" + echo " " + echo ">>> [x] [ ] Testing grb::eWiseApply using (+,0) on vectors" echo " of doubles of size 100." $runner ${TEST_BIN_DIR}/ewiseapply_${MODE}_${BACKEND} 100 &> ${TEST_OUT_DIR}/ewiseapply_${MODE}_${BACKEND}_${P}_${T} @@ -201,12 +208,6 @@ for MODE in debug ndebug; do grep 'Test OK' ${TEST_OUT_DIR}/ewiseapply_${MODE}_${BACKEND}_${P}_${T} || echo "Test FAILED" echo " " - echo ">>> [x] [ ] Testing grb::eWiseApply using + on matrices" - $runner ${TEST_BIN_DIR}/eWiseApply_matrix_${MODE}_${BACKEND} &> ${TEST_OUT_DIR}/eWiseApply_matrix_${MODE}_${BACKEND}_${P}_${T} - head -1 ${TEST_OUT_DIR}/eWiseApply_matrix_${MODE}_${BACKEND}_${P}_${T} - grep 'Test OK' ${TEST_OUT_DIR}/eWiseApply_matrix_${MODE}_${BACKEND}_${P}_${T} || echo "Test FAILED" - echo " " - echo ">>> [x] [ ] Testing grb::eWiseApply using (+,0) on vectors" echo " of doubles of size 10 000 000." $runner ${TEST_BIN_DIR}/ewiseapply_${MODE}_${BACKEND} 10000000 &> ${TEST_OUT_DIR}/ewiseapply_large_${MODE}_${BACKEND}_${P}_${T} @@ -214,6 +215,12 @@ for MODE in debug ndebug; do grep 'Test OK' ${TEST_OUT_DIR}/ewiseapply_large_${MODE}_${BACKEND}_${P}_${T} || echo "Test FAILED" echo " " + echo ">>> [x] [ ] Testing grb::eWiseApply using + on matrices" + $runner ${TEST_BIN_DIR}/eWiseApply_matrix_${MODE}_${BACKEND} &> ${TEST_OUT_DIR}/eWiseApply_matrix_${MODE}_${BACKEND}_${P}_${T} + head -1 ${TEST_OUT_DIR}/eWiseApply_matrix_${MODE}_${BACKEND}_${P}_${T} + grep 'Test OK' ${TEST_OUT_DIR}/eWiseApply_matrix_${MODE}_${BACKEND}_${P}_${T} || echo "Test FAILED" + echo " " + echo ">>> [x] [ ] Testing grb::zip on two vectors of doubles and" echo " ints of size 10 000 000." $runner ${TEST_BIN_DIR}/zip_${MODE}_${BACKEND} 10000000 &> ${TEST_OUT_DIR}/zip_large_${MODE}_${BACKEND}_${P}_${T} From b06f77660f9dbf566bed45d8ef4e88557edd3aea Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Tue, 25 Oct 2022 13:35:56 +0200 Subject: [PATCH 3/5] Internal issue #577.2 is unrelated and a metabug herewith fixed --- tests/unit/ewiseapply.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/unit/ewiseapply.cpp b/tests/unit/ewiseapply.cpp index 20fe590a1..869921174 100644 --- a/tests/unit/ewiseapply.cpp +++ b/tests/unit/ewiseapply.cpp @@ -660,10 +660,12 @@ 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 ) { + const bool halfIsOdd = ((n / 2) % 2) == 1; + if( nnz( out ) != nnz( right ) / 2 + (halfIsOdd ? 1 : 0) ) { std::cerr << "\tunexpected number of nonzeroes ( " << nnz( out ) << ", " << "expected " << nnz( right ) / 2 << " ) at subtest 22\n"; rc = FAILED; + assert( false ); // DBG } for( const auto &pair : out ) { if( pair.first < n / 2 ) { From 21c61d698f6c6e8cff82a4c18816070fc7498f5b Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Tue, 25 Oct 2022 14:18:45 +0200 Subject: [PATCH 4/5] Harden eWiseApply unit test to detect the issue reported here --- tests/unit/ewiseapply.cpp | 93 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 86 insertions(+), 7 deletions(-) diff --git a/tests/unit/ewiseapply.cpp b/tests/unit/ewiseapply.cpp index 869921174..966cfb19e 100644 --- a/tests/unit/ewiseapply.cpp +++ b/tests/unit/ewiseapply.cpp @@ -20,6 +20,7 @@ #include + using namespace grb; void grb_program( const size_t &n, grb::RC &rc ) { @@ -659,30 +660,108 @@ void grb_program( const size_t &n, grb::RC &rc ) { rc = grb::eWiseApply( out, mask, right, right, plusM ); assert( rc == SUCCESS ); + const bool halfIsOdd = ((n / 2) % 2) == 1; if( rc == SUCCESS ) { - const bool halfIsOdd = ((n / 2) % 2) == 1; - if( nnz( out ) != nnz( right ) / 2 + (halfIsOdd ? 1 : 0) ) { + const size_t expected = nnz( right ) / 2 + (halfIsOdd ? 1 : 0); + if( nnz( out ) != expected ) { std::cerr << "\tunexpected number of nonzeroes ( " << nnz( out ) << ", " - << "expected " << nnz( right ) / 2 << " ) at subtest 22\n"; + << "expected " << expected << " ) at subtest 22\n"; rc = FAILED; - assert( false ); // DBG } for( const auto &pair : out ) { if( pair.first < n / 2 ) { if( pair.first % 2 == 0 ) { if( pair.second != static_cast< double >( .5 ) ) { std::cerr << "\tunexpected entry ( " << pair.first << ", " << pair.second - << "; expected ( " << pair.first << ", 0.5 ) at subtest 22\n"; + << " ), expected ( " << pair.first << ", 0.5 ) at subtest 22\n"; rc = FAILED; } } else { std::cerr << "\tunexpected entry ( " << pair.first << ", " << pair.second - << "; expected nothing at this entry) at subtest 22\n"; + << " ), expected nothing at this entry at subtest 22\n"; + rc = FAILED; + } + } else { + std::cerr << "\tunexpected entry ( " << pair.first << ", " << pair.second + << " ), expected nothing at this index at subtest 22\n"; + rc = FAILED; + } + } + if( rc == FAILED ) { + return; + } + } else { + return; + } + + rc = clear( right ); + rc = rc ? rc : clear( left ); + rc = rc ? rc : setElement( right, 2.17, 0 ); + rc = rc ? rc : setElement( right, 2.0, n/2 ); + rc = rc ? rc : setElement( right, 3.14, n-1 ); + rc = rc ? rc : setElement( left, 1.0, n-1 ); + rc = rc ? rc : setElement( left, -1.0, 0 ); + rc = eWiseApply( out, mask, left, right, plusM ); + assert( n % 2 == 0 ); + assert( rc == SUCCESS ); + if( rc == SUCCESS ) { + const size_t expect = 1; + if( nnz( out ) != expect ) { + std::cerr << "\tunexpected number of nonzeroes ( " << nnz( out ) << ", " + << "), expected " << expect << " ) at subtest 23\n"; + rc = FAILED; + } + for( const auto &pair : out ) { + if( pair.first == 0 ) { + if( pair.second != 1.17 ) { + std::cerr << "\tunexpected entry ( " << pair.first << ", " << pair.second + << " ), expected ( " << pair.first << ", 1.17 ) at subtest 23\n"; + rc = FAILED; + } + } else { + std::cerr << "\tunexpected entry ( " << pair.first << ", " << pair.second + << " ), expected ( " << pair.first << ", " << (n/2) << " ) " + << "at subtest 23\n"; + rc = FAILED; + } + } + if( rc == FAILED ) { + return; + } + } else { + return; + } + + rc = grb::eWiseApply( out, left, right, plusM ); + assert( rc == SUCCESS ); + if( rc == SUCCESS ) { + if( nnz( out ) != 3 ) { + std::cerr << "\tunexpected number of nonzeroes ( " << nnz( out ) << ", " + << "expected " << 3 << " ) at subtest 24\n"; + rc = FAILED; + } + for( const auto &pair : out ) { + if( pair.first == 0 ) { + if( pair.second != 1.17 ) { + std::cerr << "\tunexpected entry ( " << pair.first << ", " << pair.second + << " ), expected ( " << pair.first << ", 1.17 ) at subtest 24\n"; + rc = FAILED; + } + } else if( pair.first == n / 2 ) { + if( pair.second != 2.0 ) { + std::cerr << "\tunexpected entry ( " << pair.first << ", " << pair.second + << " ), expected ( " << pair.first << ", 2.0 ) at subtest 24\n"; + rc = FAILED; + } + } else if( pair.first == n - 1 ) { + if( !utils::equals( pair.second, 4.14, 1 ) ) { + std::cerr << "\tunexpected entry ( " << pair.first << ", " << pair.second + << " ), expected ( " << pair.first << ", 4.14 ) at subtest 24\n"; rc = FAILED; } } else { std::cerr << "\tunexpected entry ( " << pair.first << ", " << pair.second - << "; expected nothing at this index) at subtest 22\n"; + << ", expected nothing at this index at subtest 24\n"; rc = FAILED; } } From 5c347de3324079783d5e2d6e5903e874504000fe Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Tue, 25 Oct 2022 14:48:35 +0200 Subject: [PATCH 5/5] Unrelated code style fix --- include/graphblas/reference/blas1.hpp | 28 +++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/include/graphblas/reference/blas1.hpp b/include/graphblas/reference/blas1.hpp index bd8856167..8ed6763a4 100644 --- a/include/graphblas/reference/blas1.hpp +++ b/include/graphblas/reference/blas1.hpp @@ -6132,25 +6132,25 @@ namespace grb { #ifdef _H_GRB_REFERENCE_OMP_BLAS1 if( z_coors.asyncAssign( index, localUpdate ) ) { #else - if( z_coors.assign( index ) ) { + if( z_coors.assign( index ) ) { #endif - typename Ring::D4 b = static_cast< typename Ring::D4 >( z[ index ] ); - (void) foldr( t, b, ring.getAdditiveOperator() ); - z[ index ] = static_cast< OutputType >( b ); - } else { - z[ index ] = static_cast< OutputType >( - static_cast< typename Ring::D4 >( t ) - ); + typename Ring::D4 b = static_cast< typename Ring::D4 >( z[ index ] ); + (void) foldr( t, b, ring.getAdditiveOperator() ); + z[ index ] = static_cast< OutputType >( b ); + } else { + z[ index ] = static_cast< OutputType >( + static_cast< typename Ring::D4 >( t ) + ); #ifdef _H_GRB_REFERENCE_OMP_BLAS1 - (void) ++asyncAssigns; - if( asyncAssigns == maxAsyncAssigns ) { - (void) z_coors.joinUpdate( localUpdate ); - asyncAssigns = 0; - } -#endif + (void) ++asyncAssigns; + if( asyncAssigns == maxAsyncAssigns ) { + (void) z_coors.joinUpdate( localUpdate ); + asyncAssigns = 0; } +#endif } } + } #ifdef _H_GRB_REFERENCE_OMP_BLAS1 while( !z_coors.joinUpdate( localUpdate ) ) {} }