From 4b42b7a73187edf417d5cff7e4856de5e18b1d1c Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Tue, 9 Aug 2022 19:36:30 +0200 Subject: [PATCH 01/13] Issue #422: add a test for grb::PinnedVector --- tests/unit/CMakeLists.txt | 4 + tests/unit/pinnedVector.cpp | 440 ++++++++++++++++++++++++++++++++++++ tests/unit/unittests.sh | 8 + 3 files changed, 452 insertions(+) create mode 100644 tests/unit/pinnedVector.cpp diff --git a/tests/unit/CMakeLists.txt b/tests/unit/CMakeLists.txt index 4afb9ba26..f6f467ae7 100644 --- a/tests/unit/CMakeLists.txt +++ b/tests/unit/CMakeLists.txt @@ -249,6 +249,10 @@ add_grb_executables( buildMatrixUnique buildMatrixUnique.cpp ADDITIONAL_LINK_LIBRARIES test_utils ) +add_grb_executables( pinnedVector pinnedVector.cpp + BACKENDS reference reference_omp bsp1d hybrid +) + # targets to list and build the test for this category get_property( unit_tests_list GLOBAL PROPERTY tests_category_unit ) add_custom_target( "list_tests_category_unit" diff --git a/tests/unit/pinnedVector.cpp b/tests/unit/pinnedVector.cpp new file mode 100644 index 000000000..0ecc5484a --- /dev/null +++ b/tests/unit/pinnedVector.cpp @@ -0,0 +1,440 @@ + +/* + * Copyright 2021 Huawei Technologies Co., Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +#include +#include + +#include +#include +#include + +#include + + +using namespace grb; +using namespace algorithms; + +enum Test { + EMPTY, + UNPOPULATED, + ZERO_CAP, + DENSE, + DENSE_CLEARED, + /** \internal Most sparse, but not totally devoid of entries */ + MOST_SPARSE, + MOST_SPARSE_CLEARED, + SPARSE_RANDOM, + /** \internal Least sparse, but not dense */ + LEAST_SPARSE +}; + +static const enum Test AllTests[] = { + EMPTY, + UNPOPULATED, + ZERO_CAP, + DENSE, + DENSE_CLEARED, + MOST_SPARSE, + MOST_SPARSE_CLEARED, + SPARSE_RANDOM, + LEAST_SPARSE +}; + +constexpr const size_t n = 100009; + +template< typename T > +struct input { + enum Test test; + T element; +}; + +template< typename T > +struct output { + RC error_code; + PinnedVector< T > vector; +}; + +template< typename T > +static inline bool checkDense( const size_t &i, const T &val, const T &chk ) { + if( i >= n ) { + std::cerr << "Nonzero with index " << i << ", while " + << "vector size is " << n << "\n"; + return false; + } + if( val != chk ) { + std::cerr << "Nonzero has unexpected value\n"; + return false; + } + return true; +} + +template< typename T > +static inline bool checkSparse( + const size_t &i, const T &val, + const T &chk, const enum Test &test +) { + if( val != chk ) { + std::cerr << "Nonzero has unexpected value\n"; + return false; + } + switch( test ) { + case MOST_SPARSE: + if( i != n/2 ) { + std::cerr << "Nonzero at position " << i << ", expected " << n/2 << "\n"; + return false; + } + break; + case SPARSE_RANDOM: + if( i > n ) { + std::cerr << "Nonzero at invalid position " << i << "; " + << "vector size is " << n << "\n"; + return false; + } + break; + case LEAST_SPARSE: + if( i == n/2 ) { + std::cerr << "Nonzero at position " << i << ", while none should be here\n"; + return false; + } + break; + default: + std::cerr << "This could should not be reached\n"; + assert( false ); + return false; + } + return true; +} + +template< typename T > +void grbProgram( const struct input< T > &in, struct output< T > &out ) { + // create container + constexpr const size_t zero = 0; + Vector< T > empty( zero ), nonempty( n ), zero_cap( n, zero ); + srand( 15124 ); + RC rc = SUCCESS; + + // print progress + switch( in.test ) { + case EMPTY: + std::cout << "\t Testing empty vectors...\n"; + break; + case UNPOPULATED: + std::cout << "\t Testing unpopulated vectors...\n"; + break; + case ZERO_CAP: + std::cout << "\t Testing zero-capacity vectors...\n"; + break; + case DENSE: + std::cout << "\t Testing dense vectors...\n"; + break; + case DENSE_CLEARED: + std::cout << "\t Testing cleared vectors...\n"; + break; + case MOST_SPARSE: + std::cout << "\t Testing sparse vector with one entry...\n"; + break; + case MOST_SPARSE_CLEARED: + std::cout << "\t Testing cleared vectors (from sparse)...\n"; + break; + case SPARSE_RANDOM: + std::cout << "\t Testing sparse vector with " + << "randomly positioned entries...\n"; + break; + case LEAST_SPARSE: + std::cout << "\t Testing sparse vector with only one unset entry...\n"; + break; + default: + assert( false ); + } + + // initialise + switch( in.test ) { + case DENSE: + case DENSE_CLEARED: + { + rc = grb::set( nonempty, in.element ); + break; + } + case MOST_SPARSE: + case MOST_SPARSE_CLEARED: + { + rc = grb::setElement( nonempty, in.element, n/2 ); + break; + } + case SPARSE_RANDOM: + { + for( size_t i = 0; i < n; ++i ) { + if( rand() % 10 == 0 ) { + rc = rc ? rc : grb::setElement( nonempty, in.element, i ); + } + } + break; + } + case LEAST_SPARSE: + { + Vector< bool > mask( n ); + rc = grb::setElement( mask, true, n/2 ); + rc = rc ? rc : grb::set< + grb::descriptors::invert_mask + >( nonempty, mask, in.element ); + break; + } + default: + assert( + in.test == EMPTY || + in.test == UNPOPULATED || + in.test == ZERO_CAP + ); + } + + // clear if requested + if( + rc == SUCCESS && ( + in.test == DENSE_CLEARED || + in.test == MOST_SPARSE_CLEARED + ) + ) { + rc = grb::clear( nonempty ); + } + + // return as a pinnedVector + if( rc == SUCCESS ) { + switch( in.test ) { + case EMPTY: + out.vector = PinnedVector< T >( empty, SEQUENTIAL ); + break; + case UNPOPULATED: + case DENSE: + case DENSE_CLEARED: + case MOST_SPARSE: + case MOST_SPARSE_CLEARED: + case SPARSE_RANDOM: + case LEAST_SPARSE: + out.vector = PinnedVector< T >( nonempty, SEQUENTIAL ); + break; + case ZERO_CAP: + out.vector = PinnedVector< T >( zero_cap, SEQUENTIAL ); + break; + default: + assert( false ); + } + } + + // done + out.error_code = rc; + return; +} + +template< typename T > +int runTests( struct input< T > &in ) { + struct output< T > out; + Launcher< AUTOMATIC > launcher; + RC rc = SUCCESS; + int offset = 0; + + // for every test + for( const auto &test : AllTests ) { + // run test + in.test = test; + rc = rc ? rc : launcher.exec( &grbProgram, in, out ); + if( out.error_code != SUCCESS ) { + return offset + 10; + } + + // check size of output vector + switch( test ) { + case EMPTY: + if( out.vector.size() != 0 ) { + std::cerr << "Empty pinned vector has nonzero size\n"; + rc = FAILED; + } + break; + case UNPOPULATED: + case ZERO_CAP: + case DENSE: + case DENSE_CLEARED: + case MOST_SPARSE: + case MOST_SPARSE_CLEARED: + case SPARSE_RANDOM: + case LEAST_SPARSE: + if( out.vector.size() != n ) { + std::cerr << "Vector does not have expected capacity\n"; + rc = FAILED; + } + break; + default: + assert( false ); + } + if( rc != SUCCESS ) { + return offset + 20; + } + + // check number of nonzeroes + switch( test ) { + case EMPTY: + case UNPOPULATED: + case ZERO_CAP: + case DENSE_CLEARED: + case MOST_SPARSE_CLEARED: + if( out.vector.nonzeroes() != 0 ) { + std::cerr << "Pinned vector has nonzeroes ( " << out.vector.nonzeroes() + << " ), but none were expected\n"; + rc = FAILED; + } + break; + case DENSE: + if( out.vector.nonzeroes() != n ) { + std::cerr << "Pinned vector has less than the expected number of " + << "nonzeroes ( " << out.vector.nonzeroes() << ", expected " << n + << " ).\n"; + rc = FAILED; + } + break; + case MOST_SPARSE: + if( out.vector.nonzeroes() != 1 ) { + std::cerr << "Pinned vector has " << out.vector.nonzeroes() + << " nonzeroes, expected 1\n"; + rc = FAILED; + } + break; + case SPARSE_RANDOM: + if( out.vector.nonzeroes() > n ) { + std::cerr << "Pinned vector has too many nonzeroes\n"; + rc = FAILED; + } + break; + case LEAST_SPARSE: + if( out.vector.nonzeroes() != n - 1 ) { + std::cerr << "Pinned vector has " << out.vector.nonzeroes() + << ", but should have " << (n-1) << "\n"; + rc = FAILED; + } + break; + default: + assert( false ); + } + if( rc != SUCCESS ) { + return offset + 30; + } + + // check nonzero contents via API + for( size_t k = 0; rc == SUCCESS && k < out.vector.nonzeroes(); ++k ) { + const size_t index = out.vector.getNonzeroIndex( k ); + const T value = out.vector.getNonzeroValue( k ); + switch( test ) { + case EMPTY: + case UNPOPULATED: + case ZERO_CAP: + case DENSE_CLEARED: + std::cerr << "Iterating over nonzeroes, while none should exist (I)\n"; + rc = FAILED; + break; + case DENSE: + if( !checkDense( index, value, in.element ) ) { + rc = FAILED; + } + break; + case MOST_SPARSE: + case SPARSE_RANDOM: + case LEAST_SPARSE: + if( !checkSparse( index, value, in.element, test ) ) { + rc = FAILED; + } + break; + default: + assert( false ); + } + + } + if( rc != SUCCESS ) { + return offset + 40; + } + + // check nonzero contents via iterator + // (TODO: this is not yet implemented in PinnedVector-- should we?) + /*for( const auto &nonzero : out.vector ) { + switch( test ) { + case EMPTY: + case UNPOPULATED: + case ZERO_CAP: + case DENSE_CLEARED: + std::cerr << "Iterating over nonzeroes, while none should exist (II)\n"; + rc = FAILED; + break; + case DENSE: + if( !checkDense( nonzero.first, nonzero.second, in.element ) ) { + rc = FAILED; + } + break; + case MOST_SPARSE: + case SPARSE_RANDOM: + case LEAST_SPARSE: + if( !checkSparse( nonzero.first, nonzero.second, in.element, test ) ) { + rc = FAILED; + } + break; + default: + assert( false ); + } + if( rc != SUCCESS ) { break; } + } + if( rc != SUCCESS ) { + return offset + 50; + }*/ + + offset += 60; + } + // done + return 0; +} + +int main( int argc, char ** argv ) { + // sanity check + if( argc != 1 ) { + std::cout << "Usage: " << argv[ 0 ] << std::endl; + return 0; + } + + std::cout << "Test executable: " << argv[ 0 ] << "\n"; + + // run some tests using a standard elementary type + std::cout << "Running test with double vector entries...\n"; + struct input< double > in_double; + in_double.element = 3.1415926535; + int error = runTests( in_double ); + + // run tests using a non-fundamental type + if( error == 0 ) { + std::cout << "Running test with std::pair vector entries...\n"; + struct input< std::pair< size_t, float > > in_pair; + in_pair.element = std::make_pair< size_t, float >( 17, -2.7 ); + error = runTests( in_pair ); + } + + // done + if( error ) { + std::cerr << std::flush; + std::cout << "Test FAILED\n" << std::endl; + return error; + } else { + std::cout << "Test OK\n" << std::endl; + return 0; + } +} + diff --git a/tests/unit/unittests.sh b/tests/unit/unittests.sh index b366382c0..c0264cca1 100755 --- a/tests/unit/unittests.sh +++ b/tests/unit/unittests.sh @@ -31,6 +31,7 @@ for MODE in debug ndebug; do ${TEST_BIN_DIR}/equals_${MODE} &> ${TEST_OUT_DIR}/equals_${MODE}.log head -1 ${TEST_OUT_DIR}/equals_${MODE}.log grep 'Test OK' ${TEST_OUT_DIR}/equals_${MODE}.log || echo "Test FAILED" + echo " " echo ">>> [x] [ ] Testing numerical addition operator over doubles" ${TEST_BIN_DIR}/add15d_${MODE} @@ -186,6 +187,13 @@ for MODE in debug ndebug; do grep 'Test OK' ${TEST_OUT_DIR}/set_${MODE}_${BACKEND}_${P}_${T}.log || echo "Test FAILED" echo " " + echo ">>> [x] [ ] Testing the grb::pinnedVector on fundamental and" + echo " non-fundamental value types." + $runner ${TEST_BIN_DIR}/pinnedVector_${MODE}_${BACKEND} &> ${TEST_OUT_DIR}/pinnedVector_${MODE}_${BACKEND}_${P}_${T}.log + head -1 ${TEST_OUT_DIR}/pinnedVector_${MODE}_${BACKEND}_${P}_${T}.log + 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 100." $runner ${TEST_BIN_DIR}/ewiseapply_${MODE}_${BACKEND} 100 &> ${TEST_OUT_DIR}/ewiseapply_${MODE}_${BACKEND}_${P}_${T} From 3ec5468bedb825c77ea04a1ec4f820439466120d Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Tue, 9 Aug 2022 19:37:34 +0200 Subject: [PATCH 02/13] Bugfix: code that handles descriptors::use_index to grb::set should never compile if size_t is not convertible into the vector data type. --- include/graphblas/bsp1d/io.hpp | 80 +++++++++++++++++++++++++--------- 1 file changed, 60 insertions(+), 20 deletions(-) diff --git a/include/graphblas/bsp1d/io.hpp b/include/graphblas/bsp1d/io.hpp index 1b01733d5..490ad44c6 100644 --- a/include/graphblas/bsp1d/io.hpp +++ b/include/graphblas/bsp1d/io.hpp @@ -431,7 +431,62 @@ namespace grb { return ret; } - /** \internal Requires no inter-process communication. */ + namespace internal { + + /** This is the variant that can handle use_index. */ + template< + Descriptor descr, + typename DataType, typename Coords, typename T + > + RC set_handle_use_index( + Vector< DataType, BSP1D, Coords > &x, + const size_t old_nnz, const T &val, + const typename std::enable_if< + std::is_convertible< size_t, DataType >::value, + void >::type * const = nullptr + ) { + if( descr & descriptors::use_index ) { + const internal::BSP1D_Data &data = internal::grb_BSP1D.cload(); + const auto p = data.P; + const auto s = data.s; + const auto n = grb::size( x ); + if( old_nnz < size( x ) ) { + internal::getCoordinates( internal::getLocal( x ) ).assignAll(); + } + return eWiseLambda( [ &x, &n, &s, &p ]( const size_t i ) { + x[ i ] = internal::Distribution< BSP1D >::local_index_to_global( + i, n, s, p + ); + }, x ); + } else { + return set< descr >( internal::getLocal( x ), val ); + } + } + + /** This is the variant that cannot handle use_index. */ + template< + Descriptor descr, + typename DataType, typename Coords, typename T + > + RC set_handle_use_index( + Vector< DataType, BSP1D, Coords > &x, + const size_t, const T &val, + const typename std::enable_if< + !std::is_convertible< size_t, DataType >::value, + void >::type * const = nullptr + ) { + static_assert( !(descr & descriptors::use_index ), + "use_index requires casting from size_t to the vector value type" ); + return set< descr >( internal::getLocal( x ), val ); + } + + } // end namespace internal + + /** + * \internal + * Requires no inter-process communication. + * \endinternal + */ template< Descriptor descr = descriptors::no_operation, typename DataType, typename Coords, @@ -442,8 +497,9 @@ namespace grb { const T val, const Phase &phase = EXECUTE, const typename std::enable_if< - !grb::is_object< T >::value, void - >::type * const = nullptr + !grb::is_object< T >::value && + std::is_convertible< T, DataType >::value, + void >::type * const = nullptr ) noexcept { const size_t n = size( x ); const size_t old_nnz = nnz( x ); @@ -466,23 +522,7 @@ namespace grb { } assert( phase == EXECUTE ); - RC ret = SUCCESS; - if( descr & descriptors::use_index ) { - const internal::BSP1D_Data &data = internal::grb_BSP1D.cload(); - const auto p = data.P; - const auto s = data.s; - const auto n = grb::size( x ); - if( old_nnz < size( x ) ) { - internal::getCoordinates( internal::getLocal( x ) ).assignAll(); - } - ret = eWiseLambda( [ &x, &n, &s, &p ]( const size_t i ) { - x[ i ] = internal::Distribution< BSP1D >::local_index_to_global( - i, n, s, p - ); - }, x ); - } else { - ret = set< descr >( internal::getLocal( x ), val ); - } + RC ret = internal::set_handle_use_index< descr >( x, old_nnz, val ); if( ret == SUCCESS ) { internal::setDense( x ); } From f69c16f562135123790158bee3db3964c8301ff0 Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Tue, 9 Aug 2022 19:38:23 +0200 Subject: [PATCH 03/13] Bugfix PinnedVector: should pin the stack of the coordinates, not the bitmask --- include/graphblas/bsp1d/pinnedvector.hpp | 16 ++++++++-------- include/graphblas/reference/pinnedvector.hpp | 9 +++++---- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/include/graphblas/bsp1d/pinnedvector.hpp b/include/graphblas/bsp1d/pinnedvector.hpp index e7026a4d1..a2dc68f05 100644 --- a/include/graphblas/bsp1d/pinnedvector.hpp +++ b/include/graphblas/bsp1d/pinnedvector.hpp @@ -55,10 +55,10 @@ namespace grb { utils::AutoDeleter< IOType > _raw_deleter; /** - * Tell the system to delete \a _buffered_coordinates only when we had its - * last reference. + * Tell the system to delete the stack of \a _buffered_coordinates only when + * we had its last reference. */ - utils::AutoDeleter< char > _assigned_deleter; + utils::AutoDeleter< char > _stack_deleter; /** A buffer of the local vector. */ IOType * _buffered_values; @@ -138,7 +138,7 @@ namespace grb { /** \internal No implementation notes. */ template< typename Coords > PinnedVector( const Vector< IOType, BSP1D, Coords > &x, const IOMode mode ) : - _raw_deleter( x._raw_deleter ), _assigned_deleter( x._assigned_deleter ), + _raw_deleter( x._raw_deleter ), _stack_deleter( x._buffer_deleter ), _buffered_values( mode == PARALLEL ? x._raw + x._offset : x._raw ), _mode( mode ), _length( x._global._coordinates.size() ) { @@ -157,7 +157,7 @@ namespace grb { /** \internal No implementation notes. */ PinnedVector( const PinnedVector< IOType, BSP1D > &other ) : _raw_deleter( other._raw_deleter ), - _assigned_deleter( other._assigned_deleter ), + _stack_deleter( other._stack_deleter ), _buffered_values( other._buffered_values ), _buffered_coordinates( other._buffered_coordinates ), _mode( other._mode ), _length( other._length ), @@ -167,7 +167,7 @@ namespace grb { /** \internal No implementation notes. */ PinnedVector( PinnedVector< IOType, BSP1D > &&other ) : _raw_deleter( other._raw_deleter ), - _assigned_deleter( other._assigned_deleter ), + _stack_deleter( other._stack_deleter ), _buffered_values( other._buffered_values ), //_buffered_coordinates uses std::move, below _mode( other._mode ), _length( other._length ), @@ -181,7 +181,7 @@ namespace grb { const PinnedVector< IOType, BSP1D > &other ) { _raw_deleter = other._raw_deleter; - _assigned_deleter = other._assigned_deleter; + _stack_deleter = other._stack_deleter; _buffered_values = other._buffered_values; _buffered_coordinates = other._buffered_coordinates; _mode = other._mode; @@ -196,7 +196,7 @@ namespace grb { PinnedVector< IOType, BSP1D > &&other ) { _raw_deleter = other._raw_deleter; - _assigned_deleter = other._assigned_deleter; + _stack_deleter = other._stack_deleter; _buffered_values = other._buffered_values; _buffered_coordinates = std::move( other._buffered_coordinates ); _mode = other._mode; diff --git a/include/graphblas/reference/pinnedvector.hpp b/include/graphblas/reference/pinnedvector.hpp index a9fb7ade6..81964cf01 100644 --- a/include/graphblas/reference/pinnedvector.hpp +++ b/include/graphblas/reference/pinnedvector.hpp @@ -50,10 +50,10 @@ namespace grb { utils::AutoDeleter< IOType > _raw_deleter; /** - * Tell the system to delete \a _buffered_mask only when we had its last - * reference. + * Tell the system to delete the stack of the \a _buffered_coordinates only + * when we had its last reference. */ - utils::AutoDeleter< char > _assigned_deleter; + utils::AutoDeleter< char > _coordinate_stack; /** A buffer of the local vector. */ IOType * _buffered_values; @@ -76,7 +76,7 @@ namespace grb { > > &x, const IOMode mode ) : - _raw_deleter( x._raw_deleter ), _assigned_deleter( x._assigned_deleter ), + _raw_deleter( x._raw_deleter ), _coordinate_stack( x._buffer_deleter ), _buffered_values( x._raw ), _buffered_coordinates( x._coordinates ) { (void)mode; // sequential and parallel IO mode are equivalent for this @@ -131,6 +131,7 @@ namespace grb { assert( _buffered_coordinates.size() > 0 ); assert( _buffered_values != nullptr ); const size_t index = getNonzeroIndex( k ); + assert( index < _buffered_coordinates.size() ); return _buffered_values[ index ]; } From ce209c2b6c67ff6696a9c3c1dd2fca6dd0979a4b Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Tue, 9 Aug 2022 20:11:51 +0200 Subject: [PATCH 04/13] Better name for the AutoDeleter corresponding to the coordinate stack --- include/graphblas/reference/pinnedvector.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/graphblas/reference/pinnedvector.hpp b/include/graphblas/reference/pinnedvector.hpp index 81964cf01..7a3332ab7 100644 --- a/include/graphblas/reference/pinnedvector.hpp +++ b/include/graphblas/reference/pinnedvector.hpp @@ -53,7 +53,7 @@ namespace grb { * Tell the system to delete the stack of the \a _buffered_coordinates only * when we had its last reference. */ - utils::AutoDeleter< char > _coordinate_stack; + utils::AutoDeleter< char > _stack_deleter; /** A buffer of the local vector. */ IOType * _buffered_values; @@ -76,7 +76,7 @@ namespace grb { > > &x, const IOMode mode ) : - _raw_deleter( x._raw_deleter ), _coordinate_stack( x._buffer_deleter ), + _raw_deleter( x._raw_deleter ), _stack_deleter( x._buffer_deleter ), _buffered_values( x._raw ), _buffered_coordinates( x._coordinates ) { (void)mode; // sequential and parallel IO mode are equivalent for this From 8d192ad0bae47c2c0ca96eb3ea0566c371b89f1a Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Wed, 10 Aug 2022 11:11:25 +0200 Subject: [PATCH 05/13] Better debug tracing and one super small perf. enhancement (do not call assignAll if already dense. While assignAll does the same check, this will save the call into that function altogether-- while in the other direction the double check usually has negligable cost). Also added a bit more code documentation. --- include/graphblas/reference/coordinates.hpp | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/include/graphblas/reference/coordinates.hpp b/include/graphblas/reference/coordinates.hpp index 02a5e652e..b3bff6563 100644 --- a/include/graphblas/reference/coordinates.hpp +++ b/include/graphblas/reference/coordinates.hpp @@ -445,7 +445,10 @@ namespace grb { * valid state. */ void rebuild( const bool dense ) noexcept { - // catch most trivial case: a vector of dimension 0, and the dense case +#ifdef _DEBUG + std::cout << "Coordinates::rebuild called with dense = " << dense << "\n"; +#endif + // catch most trivial case: a vector of dimension 0 (empty vector) if( _cap == 0 ) { return; } @@ -456,7 +459,8 @@ namespace grb { } assert( _assigned != nullptr ); #endif - if( dense ) { + // catch the other trivial-ish case (since can delegate) + if( dense && _n != _cap ) { #ifdef _DEBUG std::cout << "rebuildSparsity: dense case\n"; #endif @@ -1369,13 +1373,15 @@ namespace grb { assert( t_start < t_end ); assert( local_cur < pfBuf[ t_start + 1 ] ); for( size_t t_cur = t_start; t_cur < t_end; ++t_cur ) { + // The below is a _DEBUG statement that is extremely noisy, yet sometimes + // useful. Hence kept disabled by default. /*#ifdef _DEBUG #pragma omp critical { std::cout << "\t Thread " << t << " processing nonzero " << global_count << " / " << global_length << " using the local stack of thread " << t_cur << " starting from local index " << local_cur << ".\n"; -#endif*///DBG +#endif*/ assert( local_cur <= pfBuf[ t_cur + 1 ] - pfBuf[ t_cur ] ); StackType * __restrict__ const cur_stack = _buffer + t_cur * stack_offset + 1; From 17ccddb036680cde52dffbbbadbc2baa2d7c60a3 Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Wed, 10 Aug 2022 11:12:45 +0200 Subject: [PATCH 06/13] Bugfix in grb::clear (BSP1D) detected by the new test introduced for clearing issue #422 --- include/graphblas/bsp1d/io.hpp | 1 + 1 file changed, 1 insertion(+) diff --git a/include/graphblas/bsp1d/io.hpp b/include/graphblas/bsp1d/io.hpp index 490ad44c6..5302783e5 100644 --- a/include/graphblas/bsp1d/io.hpp +++ b/include/graphblas/bsp1d/io.hpp @@ -189,6 +189,7 @@ namespace grb { RC clear( Vector< DataType, BSP1D, Coords > &x ) noexcept { const RC ret = clear( internal::getLocal( x ) ); if( ret == SUCCESS ) { + x._became_dense = false; x._cleared = true; internal::signalLocalChange( x ); } From 09194dec1c8ac2fb22678d2e4d3856da4300dfc3 Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Wed, 10 Aug 2022 11:25:25 +0200 Subject: [PATCH 07/13] Issue #422: add another cleared test for good measure --- tests/unit/pinnedVector.cpp | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/tests/unit/pinnedVector.cpp b/tests/unit/pinnedVector.cpp index 0ecc5484a..e291974cb 100644 --- a/tests/unit/pinnedVector.cpp +++ b/tests/unit/pinnedVector.cpp @@ -41,7 +41,8 @@ enum Test { MOST_SPARSE_CLEARED, SPARSE_RANDOM, /** \internal Least sparse, but not dense */ - LEAST_SPARSE + LEAST_SPARSE, + LEAST_SPARSE_CLEARED }; static const enum Test AllTests[] = { @@ -53,7 +54,8 @@ static const enum Test AllTests[] = { MOST_SPARSE, MOST_SPARSE_CLEARED, SPARSE_RANDOM, - LEAST_SPARSE + LEAST_SPARSE, + LEAST_SPARSE_CLEARED }; constexpr const size_t n = 100009; @@ -159,6 +161,9 @@ void grbProgram( const struct input< T > &in, struct output< T > &out ) { case LEAST_SPARSE: std::cout << "\t Testing sparse vector with only one unset entry...\n"; break; + case LEAST_SPARSE_CLEARED: + std::cout << "\t Testing cleared vector (from almost-dense)...\n"; + break; default: assert( false ); } @@ -187,6 +192,7 @@ void grbProgram( const struct input< T > &in, struct output< T > &out ) { break; } case LEAST_SPARSE: + case LEAST_SPARSE_CLEARED: { Vector< bool > mask( n ); rc = grb::setElement( mask, true, n/2 ); @@ -207,7 +213,8 @@ void grbProgram( const struct input< T > &in, struct output< T > &out ) { if( rc == SUCCESS && ( in.test == DENSE_CLEARED || - in.test == MOST_SPARSE_CLEARED + in.test == MOST_SPARSE_CLEARED || + in.test == LEAST_SPARSE_CLEARED ) ) { rc = grb::clear( nonempty ); @@ -226,6 +233,7 @@ void grbProgram( const struct input< T > &in, struct output< T > &out ) { case MOST_SPARSE_CLEARED: case SPARSE_RANDOM: case LEAST_SPARSE: + case LEAST_SPARSE_CLEARED: out.vector = PinnedVector< T >( nonempty, SEQUENTIAL ); break; case ZERO_CAP: @@ -273,6 +281,7 @@ int runTests( struct input< T > &in ) { case MOST_SPARSE_CLEARED: case SPARSE_RANDOM: case LEAST_SPARSE: + case LEAST_SPARSE_CLEARED: if( out.vector.size() != n ) { std::cerr << "Vector does not have expected capacity\n"; rc = FAILED; @@ -292,6 +301,7 @@ int runTests( struct input< T > &in ) { case ZERO_CAP: case DENSE_CLEARED: case MOST_SPARSE_CLEARED: + case LEAST_SPARSE_CLEARED: if( out.vector.nonzeroes() != 0 ) { std::cerr << "Pinned vector has nonzeroes ( " << out.vector.nonzeroes() << " ), but none were expected\n"; @@ -342,6 +352,8 @@ int runTests( struct input< T > &in ) { case UNPOPULATED: case ZERO_CAP: case DENSE_CLEARED: + case MOST_SPARSE_CLEARED: + case LEAST_SPARSE_CLEARED: std::cerr << "Iterating over nonzeroes, while none should exist (I)\n"; rc = FAILED; break; @@ -374,6 +386,8 @@ int runTests( struct input< T > &in ) { case UNPOPULATED: case ZERO_CAP: case DENSE_CLEARED: + case MOST_SPARSE_CLEARED: + case LEAST_SPARSE_CLEARED: std::cerr << "Iterating over nonzeroes, while none should exist (II)\n"; rc = FAILED; break; From 6246b1d273d549f9b5721175713ee94972ee1a4a Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Wed, 10 Aug 2022 11:53:12 +0200 Subject: [PATCH 08/13] Issue #422: Now tests both SEQUENTIAL and PARALLEL modes of the pinnedVector --- tests/unit/pinnedVector.cpp | 112 +++++++++++++++++++++++++----------- 1 file changed, 77 insertions(+), 35 deletions(-) diff --git a/tests/unit/pinnedVector.cpp b/tests/unit/pinnedVector.cpp index e291974cb..5b140e442 100644 --- a/tests/unit/pinnedVector.cpp +++ b/tests/unit/pinnedVector.cpp @@ -64,6 +64,7 @@ template< typename T > struct input { enum Test test; T element; + IOMode mode; }; template< typename T > @@ -72,6 +73,17 @@ struct output { PinnedVector< T > vector; }; +struct reducer_output { + RC error_code; + size_t reduced; +}; + +void reducer( const size_t &in, struct reducer_output &out ) { + operators::add< size_t > addOp; + out.reduced = in; + out.error_code = collectives<>::allreduce( out.reduced, addOp ); +} + template< typename T > static inline bool checkDense( const size_t &i, const T &val, const T &chk ) { if( i >= n ) { @@ -134,35 +146,35 @@ void grbProgram( const struct input< T > &in, struct output< T > &out ) { // print progress switch( in.test ) { case EMPTY: - std::cout << "\t Testing empty vectors...\n"; + std::cout << "\t\t testing empty vectors...\n"; break; case UNPOPULATED: - std::cout << "\t Testing unpopulated vectors...\n"; + std::cout << "\t\t testing unpopulated vectors...\n"; break; case ZERO_CAP: - std::cout << "\t Testing zero-capacity vectors...\n"; + std::cout << "\t\t testing zero-capacity vectors...\n"; break; case DENSE: - std::cout << "\t Testing dense vectors...\n"; + std::cout << "\t\t testing dense vectors...\n"; break; case DENSE_CLEARED: - std::cout << "\t Testing cleared vectors...\n"; + std::cout << "\t\t testing cleared vectors...\n"; break; case MOST_SPARSE: - std::cout << "\t Testing sparse vector with one entry...\n"; + std::cout << "\t\t testing sparse vector with one entry...\n"; break; case MOST_SPARSE_CLEARED: - std::cout << "\t Testing cleared vectors (from sparse)...\n"; + std::cout << "\t\t testing cleared vectors (from sparse)...\n"; break; case SPARSE_RANDOM: - std::cout << "\t Testing sparse vector with " + std::cout << "\t\t testing sparse vector with " << "randomly positioned entries...\n"; break; case LEAST_SPARSE: - std::cout << "\t Testing sparse vector with only one unset entry...\n"; + std::cout << "\t\t testing sparse vector with only one unset entry...\n"; break; case LEAST_SPARSE_CLEARED: - std::cout << "\t Testing cleared vector (from almost-dense)...\n"; + std::cout << "\t\t testing cleared vector (from almost-dense)...\n"; break; default: assert( false ); @@ -224,7 +236,7 @@ void grbProgram( const struct input< T > &in, struct output< T > &out ) { if( rc == SUCCESS ) { switch( in.test ) { case EMPTY: - out.vector = PinnedVector< T >( empty, SEQUENTIAL ); + out.vector = PinnedVector< T >( empty, in.mode ); break; case UNPOPULATED: case DENSE: @@ -234,10 +246,10 @@ void grbProgram( const struct input< T > &in, struct output< T > &out ) { case SPARSE_RANDOM: case LEAST_SPARSE: case LEAST_SPARSE_CLEARED: - out.vector = PinnedVector< T >( nonempty, SEQUENTIAL ); + out.vector = PinnedVector< T >( nonempty, in.mode ); break; case ZERO_CAP: - out.vector = PinnedVector< T >( zero_cap, SEQUENTIAL ); + out.vector = PinnedVector< T >( zero_cap, in.mode ); break; default: assert( false ); @@ -294,6 +306,19 @@ int runTests( struct input< T > &in ) { return offset + 20; } + // get number of nonzeroes + size_t nzs = out.vector.nonzeroes(); + if( in.mode == PARALLEL ) { + struct reducer_output redout; + const RC reducer_error = + launcher.exec( &reducer, nzs, redout, false ); + if( reducer_error != SUCCESS || redout.error_code != SUCCESS ) { + std::cerr << "Error recovering the global number of returned nonzeroes\n"; + return offset + 25; + } + nzs = redout.reduced; + } + // check number of nonzeroes switch( test ) { case EMPTY: @@ -302,36 +327,36 @@ int runTests( struct input< T > &in ) { case DENSE_CLEARED: case MOST_SPARSE_CLEARED: case LEAST_SPARSE_CLEARED: - if( out.vector.nonzeroes() != 0 ) { - std::cerr << "Pinned vector has nonzeroes ( " << out.vector.nonzeroes() + if( nzs != 0 ) { + std::cerr << "Pinned vector has nonzeroes ( " << nzs << " ), but none were expected\n"; rc = FAILED; } break; case DENSE: - if( out.vector.nonzeroes() != n ) { - std::cerr << "Pinned vector has less than the expected number of " - << "nonzeroes ( " << out.vector.nonzeroes() << ", expected " << n + if( nzs != n ) { + std::cerr << "Pinned vector does not hold the expected number of " + << "nonzeroes ( " << nzs << ", expected " << n << " ).\n"; rc = FAILED; } break; case MOST_SPARSE: - if( out.vector.nonzeroes() != 1 ) { - std::cerr << "Pinned vector has " << out.vector.nonzeroes() + if( nzs != 1 ) { + std::cerr << "Pinned vector has " << nzs << " nonzeroes, expected 1\n"; rc = FAILED; } break; case SPARSE_RANDOM: - if( out.vector.nonzeroes() > n ) { + if( nzs > n ) { std::cerr << "Pinned vector has too many nonzeroes\n"; rc = FAILED; } break; case LEAST_SPARSE: - if( out.vector.nonzeroes() != n - 1 ) { - std::cerr << "Pinned vector has " << out.vector.nonzeroes() + if( nzs != n - 1 ) { + std::cerr << "Pinned vector has " << nzs << ", but should have " << (n-1) << "\n"; rc = FAILED; } @@ -427,18 +452,35 @@ int main( int argc, char ** argv ) { std::cout << "Test executable: " << argv[ 0 ] << "\n"; - // run some tests using a standard elementary type - std::cout << "Running test with double vector entries...\n"; - struct input< double > in_double; - in_double.element = 3.1415926535; - int error = runTests( in_double ); - - // run tests using a non-fundamental type - if( error == 0 ) { - std::cout << "Running test with std::pair vector entries...\n"; - struct input< std::pair< size_t, float > > in_pair; - in_pair.element = std::make_pair< size_t, float >( 17, -2.7 ); - error = runTests( in_pair ); + int error = 0; + const IOMode modes[] = { SEQUENTIAL, PARALLEL }; + for( const auto &mode : modes ) { + std::cout << "Testing pinnedVector in "; + if( mode == SEQUENTIAL ) { + std::cout << "SEQUENTIAL "; + } else if( mode == PARALLEL ) { + std::cout << "PARALLEL "; + } else { + assert( false ); + } + std::cout << "I/O mode\n"; + + // run some tests using a standard elementary type + std::cout << "\t running tests with double vector entries...\n"; + struct input< double > in_double; + in_double.element = 3.1415926535; + in_double.mode = mode; + error = runTests( in_double ); + + // run tests using a non-fundamental type + if( error == 0 ) { + std::cout << "\t running tests with std::pair vector entries...\n"; + struct input< std::pair< size_t, float > > in_pair; + in_pair.element = std::make_pair< size_t, float >( 17, -2.7 ); + in_pair.mode = mode; + error = runTests( in_pair ); + } + if( error ) { break; } } // done From fc072184ff55ae568782e442add0b8e8fd9dd274 Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Wed, 10 Aug 2022 11:53:32 +0200 Subject: [PATCH 09/13] Unrelated minor code style fixes --- include/graphblas/bsp1d/exec.hpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/include/graphblas/bsp1d/exec.hpp b/include/graphblas/bsp1d/exec.hpp index 8c60cc864..b520ada6d 100644 --- a/include/graphblas/bsp1d/exec.hpp +++ b/include/graphblas/bsp1d/exec.hpp @@ -37,9 +37,10 @@ #include "init.hpp" #ifndef _GRB_NO_STDIO -#include //for std::cerr + #include //for std::cerr #endif + /** Global internal singleton to track whether MPI was initialized. */ extern bool _grb_mpi_initialized; @@ -51,8 +52,8 @@ void _grb_exec_spmd( lpf_t ctx, lpf_pid_t s, lpf_pid_t P, lpf_args_t args ) { #ifdef _DEBUG if( s == 0 ) { - std::cout << "Info: launcher spawned or hooked " << P - << " ALP/GraphBLAS user processes.\n"; + std::cout << "Info: launcher spawned or hooked " << P << " ALP/GraphBLAS " + << "user processes.\n"; } #endif From 5d11833eb2a6fe8a00e1e1982051b37a6d53c18b Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Wed, 10 Aug 2022 12:24:09 +0200 Subject: [PATCH 10/13] When testing PARALLEL I/O but launching from a sequential process, then of course we only get one part of the vector for P>1. If we want to perform a test alike the one now removed, then a new natively parallel test (using either MPI or LPF and using Launcher< MANUAL >) must be constructed instead. --- tests/unit/pinnedVector.cpp | 42 ++++++++++++++++--------------------- 1 file changed, 18 insertions(+), 24 deletions(-) diff --git a/tests/unit/pinnedVector.cpp b/tests/unit/pinnedVector.cpp index 5b140e442..22e86184a 100644 --- a/tests/unit/pinnedVector.cpp +++ b/tests/unit/pinnedVector.cpp @@ -73,17 +73,6 @@ struct output { PinnedVector< T > vector; }; -struct reducer_output { - RC error_code; - size_t reduced; -}; - -void reducer( const size_t &in, struct reducer_output &out ) { - operators::add< size_t > addOp; - out.reduced = in; - out.error_code = collectives<>::allreduce( out.reduced, addOp ); -} - template< typename T > static inline bool checkDense( const size_t &i, const T &val, const T &chk ) { if( i >= n ) { @@ -308,16 +297,6 @@ int runTests( struct input< T > &in ) { // get number of nonzeroes size_t nzs = out.vector.nonzeroes(); - if( in.mode == PARALLEL ) { - struct reducer_output redout; - const RC reducer_error = - launcher.exec( &reducer, nzs, redout, false ); - if( reducer_error != SUCCESS || redout.error_code != SUCCESS ) { - std::cerr << "Error recovering the global number of returned nonzeroes\n"; - return offset + 25; - } - nzs = redout.reduced; - } // check number of nonzeroes switch( test ) { @@ -334,19 +313,29 @@ int runTests( struct input< T > &in ) { } break; case DENSE: - if( nzs != n ) { + if( in.mode == SEQUENTIAL && nzs != n ) { std::cerr << "Pinned vector does not hold the expected number of " << "nonzeroes ( " << nzs << ", expected " << n << " ).\n"; rc = FAILED; } + if( in.mode == PARALLEL && nzs > n ) { + std::cerr << "Pinned vector holds too many nonzeroes ( " << nzs << ", " + << "maximum is " << n << " ).\n"; + rc = FAILED; + } break; case MOST_SPARSE: - if( nzs != 1 ) { + if( in.mode == SEQUENTIAL && nzs != 1 ) { std::cerr << "Pinned vector has " << nzs << " nonzeroes, expected 1\n"; rc = FAILED; } + if( in.mode == PARALLEL && nzs > 1 ) { + std::cerr << "Pinned vector holds too many nonzeroes ( " << nzs << ", " + << "maximum is 1 ).\n"; + rc = FAILED; + } break; case SPARSE_RANDOM: if( nzs > n ) { @@ -355,11 +344,16 @@ int runTests( struct input< T > &in ) { } break; case LEAST_SPARSE: - if( nzs != n - 1 ) { + if( in.mode == SEQUENTIAL && nzs != n - 1 ) { std::cerr << "Pinned vector has " << nzs << ", but should have " << (n-1) << "\n"; rc = FAILED; } + if( in.mode == PARALLEL && nzs > n - 1 ) { + std::cerr << "Pinned vector holds too many nonzeroes ( " << nzs << ", " + << "maximum is " << (n-1) << " ).\n"; + rc = FAILED; + } break; default: assert( false ); From 660c995a62147ac61f11ec3e8ad4783e65bb44f3 Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Wed, 10 Aug 2022 12:30:45 +0200 Subject: [PATCH 11/13] Add a new test on vectors with two entries only --- tests/unit/pinnedVector.cpp | 36 ++++++++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/tests/unit/pinnedVector.cpp b/tests/unit/pinnedVector.cpp index 22e86184a..a1986856a 100644 --- a/tests/unit/pinnedVector.cpp +++ b/tests/unit/pinnedVector.cpp @@ -39,6 +39,7 @@ enum Test { /** \internal Most sparse, but not totally devoid of entries */ MOST_SPARSE, MOST_SPARSE_CLEARED, + TWO_ENTRIES, SPARSE_RANDOM, /** \internal Least sparse, but not dense */ LEAST_SPARSE, @@ -53,6 +54,7 @@ static const enum Test AllTests[] = { DENSE_CLEARED, MOST_SPARSE, MOST_SPARSE_CLEARED, + TWO_ENTRIES, SPARSE_RANDOM, LEAST_SPARSE, LEAST_SPARSE_CLEARED @@ -103,6 +105,13 @@ static inline bool checkSparse( return false; } break; + case TWO_ENTRIES: + if( i != 0 && i != n-1 ) { + std::cerr << "Unexpected nonzero at position " << i << ", expected 0 or " + << (n-1) << " only.\n"; + return false; + } + break; case SPARSE_RANDOM: if( i > n ) { std::cerr << "Nonzero at invalid position " << i << "; " @@ -152,6 +161,9 @@ void grbProgram( const struct input< T > &in, struct output< T > &out ) { case MOST_SPARSE: std::cout << "\t\t testing sparse vector with one entry...\n"; break; + case TWO_ENTRIES: + std::cout << "\t\t testing sparse vector with two entries...\n"; + break; case MOST_SPARSE_CLEARED: std::cout << "\t\t testing cleared vectors (from sparse)...\n"; break; @@ -183,6 +195,12 @@ void grbProgram( const struct input< T > &in, struct output< T > &out ) { rc = grb::setElement( nonempty, in.element, n/2 ); break; } + case TWO_ENTRIES: + { + rc = grb::setElement( nonempty, in.element, 0 ); + rc = rc ? rc : grb::setElement( nonempty, in.element, n-1 ); + break; + } case SPARSE_RANDOM: { for( size_t i = 0; i < n; ++i ) { @@ -232,6 +250,7 @@ void grbProgram( const struct input< T > &in, struct output< T > &out ) { case DENSE_CLEARED: case MOST_SPARSE: case MOST_SPARSE_CLEARED: + case TWO_ENTRIES: case SPARSE_RANDOM: case LEAST_SPARSE: case LEAST_SPARSE_CLEARED: @@ -280,6 +299,7 @@ int runTests( struct input< T > &in ) { case DENSE_CLEARED: case MOST_SPARSE: case MOST_SPARSE_CLEARED: + case TWO_ENTRIES: case SPARSE_RANDOM: case LEAST_SPARSE: case LEAST_SPARSE_CLEARED: @@ -327,8 +347,7 @@ int runTests( struct input< T > &in ) { break; case MOST_SPARSE: if( in.mode == SEQUENTIAL && nzs != 1 ) { - std::cerr << "Pinned vector has " << nzs - << " nonzeroes, expected 1\n"; + std::cerr << "Pinned vector has " << nzs << " nonzeroes, expected 1\n"; rc = FAILED; } if( in.mode == PARALLEL && nzs > 1 ) { @@ -337,6 +356,17 @@ int runTests( struct input< T > &in ) { rc = FAILED; } break; + case TWO_ENTRIES: + if( in.mode == SEQUENTIAL && nzs != 2 ) { + std::cerr << "Pinned vector has " << nzs << " nonzeroes, expected 2\n"; + rc = FAILED; + } + if( in.mode == PARALLEL && nzs > 2 ) { + std::cerr << "Pinned vector holds too many nonzeroes ( " << nzs << ", " + << "maximum is 1 ).\n"; + rc = FAILED; + } + break; case SPARSE_RANDOM: if( nzs > n ) { std::cerr << "Pinned vector has too many nonzeroes\n"; @@ -382,6 +412,7 @@ int runTests( struct input< T > &in ) { } break; case MOST_SPARSE: + case TWO_ENTRIES: case SPARSE_RANDOM: case LEAST_SPARSE: if( !checkSparse( index, value, in.element, test ) ) { @@ -416,6 +447,7 @@ int runTests( struct input< T > &in ) { } break; case MOST_SPARSE: + case TWO_ENTRIES: case SPARSE_RANDOM: case LEAST_SPARSE: if( !checkSparse( nonzero.first, nonzero.second, in.element, test ) ) { From 61d95e56e351ef9f5136bd8b6fb36f2cf5cbfa78 Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Wed, 10 Aug 2022 12:33:48 +0200 Subject: [PATCH 12/13] Code robustness: BSP1D PinnedVector constructor now throws on errors. Also in PARALLEL mode, if the nnz count is not updated, do that before pinning. --- include/graphblas/bsp1d/pinnedvector.hpp | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/include/graphblas/bsp1d/pinnedvector.hpp b/include/graphblas/bsp1d/pinnedvector.hpp index a2dc68f05..cfe4fc323 100644 --- a/include/graphblas/bsp1d/pinnedvector.hpp +++ b/include/graphblas/bsp1d/pinnedvector.hpp @@ -147,9 +147,20 @@ namespace grb { _P = data.P; if( mode != PARALLEL ) { assert( mode == SEQUENTIAL ); - x.synchronize(); + const RC rc = x.synchronize(); + if( rc != SUCCESS ) { + throw std::runtime_error( + "Could not synchronise vector during pinning: " + toString( rc ) + ); + } _buffered_coordinates = x._global._coordinates; } else { + const RC rc = x.updateNnz(); + if( rc != SUCCESS ) { + throw std::runtime_error( + "Could not update vector nonzero count during pinning: " + toString( rc ) + ); + } _buffered_coordinates = x._local._coordinates; } } From 8277de4b2f4e840edabf4c084567a0afaea96c69 Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Wed, 10 Aug 2022 12:50:04 +0200 Subject: [PATCH 13/13] The test now does size checking the moment the PinnedVector is created in the ALP/GraphBLAS context itself. It also does number-of-nonzeroes checking within the ALP/GraphBLAS context when in parallel mode, since this is the only place that is possible when using the launcher in automatic mode --- tests/unit/pinnedVector.cpp | 64 ++++++++++++++++++++++++++++++++++++- 1 file changed, 63 insertions(+), 1 deletion(-) diff --git a/tests/unit/pinnedVector.cpp b/tests/unit/pinnedVector.cpp index a1986856a..d9bf070e4 100644 --- a/tests/unit/pinnedVector.cpp +++ b/tests/unit/pinnedVector.cpp @@ -244,6 +244,9 @@ void grbProgram( const struct input< T > &in, struct output< T > &out ) { switch( in.test ) { case EMPTY: out.vector = PinnedVector< T >( empty, in.mode ); + if( out.vector.size() != 0 ) { + rc = FAILED; + } break; case UNPOPULATED: case DENSE: @@ -255,13 +258,72 @@ void grbProgram( const struct input< T > &in, struct output< T > &out ) { case LEAST_SPARSE: case LEAST_SPARSE_CLEARED: out.vector = PinnedVector< T >( nonempty, in.mode ); + if( out.vector.size() != n ) { + rc = FAILED; + } break; case ZERO_CAP: out.vector = PinnedVector< T >( zero_cap, in.mode ); - break; + if( out.vector.size() != n ) { + rc = FAILED; + } + break; default: assert( false ); } + if( rc == FAILED ) { + std::cerr << "To-be returned PinnedVector has invalid size " + << out.vector.size() << "\n"; + return; + } + } + + // check total number of nozeroes when in PARALLEL mode + if( rc == SUCCESS && in.mode == PARALLEL ) { + operators::add< size_t > addOp; + size_t nzs = out.vector.nonzeroes(); + if( collectives<>::allreduce( nzs, addOp ) != SUCCESS ) { + std::cerr << "Could not reduce the number of nonzeroes of all pinned " + << "vectors.\n"; + rc = FAILED; + return; + } + size_t expect = 0; + switch( in.test ) { + case EMPTY: + case UNPOPULATED: + case ZERO_CAP: + case DENSE_CLEARED: + case MOST_SPARSE_CLEARED: + case LEAST_SPARSE_CLEARED: + expect = 0; + break; + case DENSE: + expect = n; + break; + case MOST_SPARSE: + expect = 1; + break; + case TWO_ENTRIES: + expect = 2; + break; + case SPARSE_RANDOM: + break; + case LEAST_SPARSE: + expect = n-1; + break; + default: + assert( false ); + } + if( in.test != SPARSE_RANDOM && nzs != expect ) { + std::cerr << "Expected " << expect << " nonzeroes, but " + << nzs << " nonzeroes found\n"; + rc = FAILED; + } + if( in.test == SPARSE_RANDOM && nzs > n ) { + std::cerr << "Illegal number of nonzeroes " << nzs << "\n"; + rc = FAILED; + } } // done