Skip to content

Commit a1dddb1

Browse files
anyzelmananhansson
anyzelman
authored and
anhansson
committed
Resolves internal issue #577: two bugs related to eWiseApply (#115)
Aristeidis detected that the unit test for `eWiseApply` could fail for some vector lengths that `unittests.sh` never calls it with, and also detected that the sparse-vector driven variant of `eWiseApply` did not correctly check for output mask conditions. The underlying issues were a metabug in the unit test that did not deal properly with vector lengths whose half is an odd number, and a missing negation of the evaluated mask. This MR fixes both issues, and also: - executes the `eWiseApply` unit test for vectors of length `14`, thus triggering the detecting meta-bug should it ever regress; - adds to sub-tests to the `eWiseApply` unit test designed to trigger the sparse-vector driven variant with a non-trivial mask that would detect erroneous mask evaluations, thus triggering detection of the main bug this MR fixes; - improved the output of the `eWiseApply` unit tests; and - fixes an unrelated code style issue (incorrect indentation).
1 parent 738cbe0 commit a1dddb1

File tree

3 files changed

+115
-27
lines changed

3 files changed

+115
-27
lines changed

‎include/graphblas/reference/blas1.hpp‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3306,7 +3306,7 @@ namespace grb {
33063306
#endif
33073307
for( ; k < loop_coors.nonzeroes(); ++k ) {
33083308
const size_t index = loop_coors.index( k );
3309-
if( masked && mask_coors->template mask< descr >( index, mask_p ) ) {
3309+
if( masked && !mask_coors->template mask< descr >( index, mask_p ) ) {
33103310
continue;
33113311
}
33123312
RC rc = SUCCESS;
@@ -3518,7 +3518,7 @@ namespace grb {
35183518
if( loop_coors.assigned( index ) ) {
35193519
continue;
35203520
}
3521-
if( masked && mask_coors->template mask< descr >( index, mask_p ) ) {
3521+
if( masked && !mask_coors->template mask< descr >( index, mask_p ) ) {
35223522
continue;
35233523
}
35243524
#ifndef _H_GRB_REFERENCE_OMP_BLAS1
@@ -6132,25 +6132,25 @@ namespace grb {
61326132
#ifdef _H_GRB_REFERENCE_OMP_BLAS1
61336133
if( z_coors.asyncAssign( index, localUpdate ) ) {
61346134
#else
6135-
if( z_coors.assign( index ) ) {
6135+
if( z_coors.assign( index ) ) {
61366136
#endif
6137-
typename Ring::D4 b = static_cast< typename Ring::D4 >( z[ index ] );
6138-
(void) foldr( t, b, ring.getAdditiveOperator() );
6139-
z[ index ] = static_cast< OutputType >( b );
6140-
} else {
6141-
z[ index ] = static_cast< OutputType >(
6142-
static_cast< typename Ring::D4 >( t )
6143-
);
6137+
typename Ring::D4 b = static_cast< typename Ring::D4 >( z[ index ] );
6138+
(void) foldr( t, b, ring.getAdditiveOperator() );
6139+
z[ index ] = static_cast< OutputType >( b );
6140+
} else {
6141+
z[ index ] = static_cast< OutputType >(
6142+
static_cast< typename Ring::D4 >( t )
6143+
);
61446144
#ifdef _H_GRB_REFERENCE_OMP_BLAS1
6145-
(void) ++asyncAssigns;
6146-
if( asyncAssigns == maxAsyncAssigns ) {
6147-
(void) z_coors.joinUpdate( localUpdate );
6148-
asyncAssigns = 0;
6149-
}
6150-
#endif
6145+
(void) ++asyncAssigns;
6146+
if( asyncAssigns == maxAsyncAssigns ) {
6147+
(void) z_coors.joinUpdate( localUpdate );
6148+
asyncAssigns = 0;
61516149
}
6150+
#endif
61526151
}
61536152
}
6153+
}
61546154
#ifdef _H_GRB_REFERENCE_OMP_BLAS1
61556155
while( !z_coors.joinUpdate( localUpdate ) ) {}
61566156
}

‎tests/unit/ewiseapply.cpp‎

Lines changed: 86 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020

2121
#include <graphblas.hpp>
2222

23+
2324
using namespace grb;
2425

2526
void grb_program( const size_t &n, grb::RC &rc ) {
@@ -659,28 +660,108 @@ void grb_program( const size_t &n, grb::RC &rc ) {
659660

660661
rc = grb::eWiseApply( out, mask, right, right, plusM );
661662
assert( rc == SUCCESS );
663+
const bool halfIsOdd = ((n / 2) % 2) == 1;
662664
if( rc == SUCCESS ) {
663-
if( nnz( out ) != nnz( right ) / 2 ) {
665+
const size_t expected = nnz( right ) / 2 + (halfIsOdd ? 1 : 0);
666+
if( nnz( out ) != expected ) {
664667
std::cerr << "\tunexpected number of nonzeroes ( " << nnz( out ) << ", "
665-
<< "expected " << nnz( right ) / 2 << " ) at subtest 22\n";
668+
<< "expected " << expected << " ) at subtest 22\n";
666669
rc = FAILED;
667670
}
668671
for( const auto &pair : out ) {
669672
if( pair.first < n / 2 ) {
670673
if( pair.first % 2 == 0 ) {
671674
if( pair.second != static_cast< double >( .5 ) ) {
672675
std::cerr << "\tunexpected entry ( " << pair.first << ", " << pair.second
673-
<< "; expected ( " << pair.first << ", 0.5 ) at subtest 22\n";
676+
<< " ), expected ( " << pair.first << ", 0.5 ) at subtest 22\n";
674677
rc = FAILED;
675678
}
676679
} else {
677680
std::cerr << "\tunexpected entry ( " << pair.first << ", " << pair.second
678-
<< "; expected nothing at this entry) at subtest 22\n";
681+
<< " ), expected nothing at this entry at subtest 22\n";
682+
rc = FAILED;
683+
}
684+
} else {
685+
std::cerr << "\tunexpected entry ( " << pair.first << ", " << pair.second
686+
<< " ), expected nothing at this index at subtest 22\n";
687+
rc = FAILED;
688+
}
689+
}
690+
if( rc == FAILED ) {
691+
return;
692+
}
693+
} else {
694+
return;
695+
}
696+
697+
rc = clear( right );
698+
rc = rc ? rc : clear( left );
699+
rc = rc ? rc : setElement( right, 2.17, 0 );
700+
rc = rc ? rc : setElement( right, 2.0, n/2 );
701+
rc = rc ? rc : setElement( right, 3.14, n-1 );
702+
rc = rc ? rc : setElement( left, 1.0, n-1 );
703+
rc = rc ? rc : setElement( left, -1.0, 0 );
704+
rc = eWiseApply( out, mask, left, right, plusM );
705+
assert( n % 2 == 0 );
706+
assert( rc == SUCCESS );
707+
if( rc == SUCCESS ) {
708+
const size_t expect = 1;
709+
if( nnz( out ) != expect ) {
710+
std::cerr << "\tunexpected number of nonzeroes ( " << nnz( out ) << ", "
711+
<< "), expected " << expect << " ) at subtest 23\n";
712+
rc = FAILED;
713+
}
714+
for( const auto &pair : out ) {
715+
if( pair.first == 0 ) {
716+
if( pair.second != 1.17 ) {
717+
std::cerr << "\tunexpected entry ( " << pair.first << ", " << pair.second
718+
<< " ), expected ( " << pair.first << ", 1.17 ) at subtest 23\n";
719+
rc = FAILED;
720+
}
721+
} else {
722+
std::cerr << "\tunexpected entry ( " << pair.first << ", " << pair.second
723+
<< " ), expected ( " << pair.first << ", " << (n/2) << " ) "
724+
<< "at subtest 23\n";
725+
rc = FAILED;
726+
}
727+
}
728+
if( rc == FAILED ) {
729+
return;
730+
}
731+
} else {
732+
return;
733+
}
734+
735+
rc = grb::eWiseApply( out, left, right, plusM );
736+
assert( rc == SUCCESS );
737+
if( rc == SUCCESS ) {
738+
if( nnz( out ) != 3 ) {
739+
std::cerr << "\tunexpected number of nonzeroes ( " << nnz( out ) << ", "
740+
<< "expected " << 3 << " ) at subtest 24\n";
741+
rc = FAILED;
742+
}
743+
for( const auto &pair : out ) {
744+
if( pair.first == 0 ) {
745+
if( pair.second != 1.17 ) {
746+
std::cerr << "\tunexpected entry ( " << pair.first << ", " << pair.second
747+
<< " ), expected ( " << pair.first << ", 1.17 ) at subtest 24\n";
748+
rc = FAILED;
749+
}
750+
} else if( pair.first == n / 2 ) {
751+
if( pair.second != 2.0 ) {
752+
std::cerr << "\tunexpected entry ( " << pair.first << ", " << pair.second
753+
<< " ), expected ( " << pair.first << ", 2.0 ) at subtest 24\n";
754+
rc = FAILED;
755+
}
756+
} else if( pair.first == n - 1 ) {
757+
if( !utils::equals( pair.second, 4.14, 1 ) ) {
758+
std::cerr << "\tunexpected entry ( " << pair.first << ", " << pair.second
759+
<< " ), expected ( " << pair.first << ", 4.14 ) at subtest 24\n";
679760
rc = FAILED;
680761
}
681762
} else {
682763
std::cerr << "\tunexpected entry ( " << pair.first << ", " << pair.second
683-
<< "; expected nothing at this index) at subtest 22\n";
764+
<< ", expected nothing at this index at subtest 24\n";
684765
rc = FAILED;
685766
}
686767
}

‎tests/unit/unittests.sh‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -194,26 +194,33 @@ for MODE in debug ndebug; do
194194
grep 'Test OK' ${TEST_OUT_DIR}/pinnedVector_${MODE}_${BACKEND}_${P}_${T}.log || echo "Test FAILED"
195195
echo " "
196196

197+
echo ">>> [x] [ ] Testing grb::eWiseApply using (+,0) on vectors"
198+
echo " of doubles of size 14."
199+
$runner ${TEST_BIN_DIR}/ewiseapply_${MODE}_${BACKEND} 14 &> ${TEST_OUT_DIR}/ewiseapply_small_${MODE}_${BACKEND}_${P}_${T}
200+
head -1 ${TEST_OUT_DIR}/ewiseapply_small_${MODE}_${BACKEND}_${P}_${T}
201+
grep 'Test OK' ${TEST_OUT_DIR}/ewiseapply_small_${MODE}_${BACKEND}_${P}_${T} || echo "Test FAILED"
202+
echo " "
203+
197204
echo ">>> [x] [ ] Testing grb::eWiseApply using (+,0) on vectors"
198205
echo " of doubles of size 100."
199206
$runner ${TEST_BIN_DIR}/ewiseapply_${MODE}_${BACKEND} 100 &> ${TEST_OUT_DIR}/ewiseapply_${MODE}_${BACKEND}_${P}_${T}
200207
head -1 ${TEST_OUT_DIR}/ewiseapply_${MODE}_${BACKEND}_${P}_${T}
201208
grep 'Test OK' ${TEST_OUT_DIR}/ewiseapply_${MODE}_${BACKEND}_${P}_${T} || echo "Test FAILED"
202209
echo " "
203210

204-
echo ">>> [x] [ ] Testing grb::eWiseApply using + on matrices"
205-
$runner ${TEST_BIN_DIR}/eWiseApply_matrix_${MODE}_${BACKEND} &> ${TEST_OUT_DIR}/eWiseApply_matrix_${MODE}_${BACKEND}_${P}_${T}
206-
head -1 ${TEST_OUT_DIR}/eWiseApply_matrix_${MODE}_${BACKEND}_${P}_${T}
207-
grep 'Test OK' ${TEST_OUT_DIR}/eWiseApply_matrix_${MODE}_${BACKEND}_${P}_${T} || echo "Test FAILED"
208-
echo " "
209-
210211
echo ">>> [x] [ ] Testing grb::eWiseApply using (+,0) on vectors"
211212
echo " of doubles of size 10 000 000."
212213
$runner ${TEST_BIN_DIR}/ewiseapply_${MODE}_${BACKEND} 10000000 &> ${TEST_OUT_DIR}/ewiseapply_large_${MODE}_${BACKEND}_${P}_${T}
213214
head -1 ${TEST_OUT_DIR}/ewiseapply_large_${MODE}_${BACKEND}_${P}_${T}
214215
grep 'Test OK' ${TEST_OUT_DIR}/ewiseapply_large_${MODE}_${BACKEND}_${P}_${T} || echo "Test FAILED"
215216
echo " "
216217

218+
echo ">>> [x] [ ] Testing grb::eWiseApply using + on matrices"
219+
$runner ${TEST_BIN_DIR}/eWiseApply_matrix_${MODE}_${BACKEND} &> ${TEST_OUT_DIR}/eWiseApply_matrix_${MODE}_${BACKEND}_${P}_${T}
220+
head -1 ${TEST_OUT_DIR}/eWiseApply_matrix_${MODE}_${BACKEND}_${P}_${T}
221+
grep 'Test OK' ${TEST_OUT_DIR}/eWiseApply_matrix_${MODE}_${BACKEND}_${P}_${T} || echo "Test FAILED"
222+
echo " "
223+
217224
echo ">>> [x] [ ] Testing grb::zip on two vectors of doubles and"
218225
echo " ints of size 10 000 000."
219226
$runner ${TEST_BIN_DIR}/zip_${MODE}_${BACKEND} 10000000 &> ${TEST_OUT_DIR}/zip_large_${MODE}_${BACKEND}_${P}_${T}

0 commit comments

Comments
 (0)