From 56f0ace69f94a9296eb35b900c50f76cee747650 Mon Sep 17 00:00:00 2001 From: Benjamin Lozes Date: Wed, 19 Jul 2023 14:04:38 +0200 Subject: [PATCH 01/13] Signature declaration in base --- include/graphblas/base/vector.hpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/include/graphblas/base/vector.hpp b/include/graphblas/base/vector.hpp index c00ca6e53..3aabe66a7 100644 --- a/include/graphblas/base/vector.hpp +++ b/include/graphblas/base/vector.hpp @@ -236,6 +236,14 @@ namespace grb { (void) n; } + /** + * Creates a dense ALP/GraphBLAS vector. This constructor takes an initializer + * list of values that will be copied into this vector. + */ + Vector( const std::initializer_list< D > &vals ) { + (void) vals; + } + /** * Move constructor. * From b8640626e62f146350bd7e3234b82ef523bd836a Mon Sep 17 00:00:00 2001 From: Benjamin Lozes Date: Wed, 19 Jul 2023 14:08:55 +0200 Subject: [PATCH 02/13] Unit-test implementation --- tests/unit/CMakeLists.txt | 4 ++ tests/unit/unittests.sh | 6 ++ tests/unit/vectorFromListConstructor.cpp | 73 ++++++++++++++++++++++++ 3 files changed, 83 insertions(+) create mode 100644 tests/unit/vectorFromListConstructor.cpp diff --git a/tests/unit/CMakeLists.txt b/tests/unit/CMakeLists.txt index 64a31013b..ab0dac498 100644 --- a/tests/unit/CMakeLists.txt +++ b/tests/unit/CMakeLists.txt @@ -300,6 +300,10 @@ add_grb_executables( adapterIterator adapterIterator.cpp BACKENDS reference reference_omp hyperdags nonblocking bsp1d hybrid ) +add_grb_executables( vectorFromListConstructor vectorFromListConstructor.cpp + BACKENDS reference reference_omp bsp1d hybrid hyperdags nonblocking +) + # the below targets test successfully when they compile -- they do not need to # be executed successfully as part of the unit test suite. diff --git a/tests/unit/unittests.sh b/tests/unit/unittests.sh index 4655ee309..5b7dfc16a 100755 --- a/tests/unit/unittests.sh +++ b/tests/unit/unittests.sh @@ -407,6 +407,12 @@ for MODE in ${MODES}; do grep 'Test OK' ${TEST_OUT_DIR}/buildVector_${MODE}_${BACKEND}_${P}_${T}.log || echo "Test FAILED" echo " " + echo ">>> [x] [ ] Testing grb::Vector( initializer_list ) constructor" + $runner ${TEST_BIN_DIR}/vectorFromListConstructor_${MODE}_${BACKEND} &> ${TEST_OUT_DIR}/vectorFromListConstructor_${MODE}_${BACKEND}_${P}_${T}.log + head -1 ${TEST_OUT_DIR}/vectorFromListConstructor_${MODE}_${BACKEND}_${P}_${T}.log + grep 'Test OK' ${TEST_OUT_DIR}/vectorFromListConstructor_${MODE}_${BACKEND}_${P}_${T}.log || echo "Test FAILED" + echo " " + echo ">>> [x] [ ] Testing grb::vectorToMatrixConverter" $runner ${TEST_BIN_DIR}/vectorToMatrix_${MODE}_${BACKEND} &> ${TEST_OUT_DIR}/vectorToMatrix_${MODE}_${BACKEND}_${P}_${T}.log head -1 ${TEST_OUT_DIR}/vectorToMatrix_${MODE}_${BACKEND}_${P}_${T}.log diff --git a/tests/unit/vectorFromListConstructor.cpp b/tests/unit/vectorFromListConstructor.cpp new file mode 100644 index 000000000..6dc55ce96 --- /dev/null +++ b/tests/unit/vectorFromListConstructor.cpp @@ -0,0 +1,73 @@ + +/* + * 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 "graphblas.hpp" + +using namespace grb; + +void grbProgram( const void *, const size_t in_size, int & error ) { + error = 0; + + if( in_size != 0 ) { + std::cerr << "Unit tests called with unexpected input" << std::endl; + error = 1; + return; + } + std::vector< int > values = { 4, 7, 4, 6, 4, 7, 1, 7, 3, 6, 7, 5, 1, 8, 7 }; + grb::Vector< int > x( { 4, 7, 4, 6, 4, 7, 1, 7, 3, 6, 7, 5, 1, 8, 7 } ); + bool equals = true; + auto vector_it = x.begin(); + for( size_t i = 0; i < values.size(); i++ ) { + auto position = vector_it->first; + auto value = vector_it->second; + if( i != position || values[ i ] != value ) { + std::cerr << "Expected position " << i << " value " << values[ i ] << " but got position " << position << " value " << value << std::endl; + equals = false; + break; + } + vector_it.operator++(); + } + if( !equals ) { + std::cerr << "Vector values are not correct" << std::endl; + error = 1; + return; + } +} + +int main( int argc, char ** argv ) { + (void)argc; + std::cout << "Functional test executable: " << argv[ 0 ] << std::endl; + + int error; + grb::Launcher< AUTOMATIC > launcher; + if( launcher.exec( &grbProgram, NULL, 0, error ) != SUCCESS ) { + std::cout << "Test FAILED (test failed to launch)" << std::endl; + error = 255; + } + if( error == 0 ) { + std::cout << "Test OK" << std::endl; + } else { + std::cout << "Test FAILED" << std::endl; + } + + // done + return error; +} + From 51ad01e173fbc12661ad2803571a8616c098924b Mon Sep 17 00:00:00 2001 From: Benjamin Lozes Date: Wed, 19 Jul 2023 14:09:18 +0200 Subject: [PATCH 03/13] Implementation in reference backend --- include/graphblas/reference/vector.hpp | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/include/graphblas/reference/vector.hpp b/include/graphblas/reference/vector.hpp index 7dd5f202e..dd016bab6 100644 --- a/include/graphblas/reference/vector.hpp +++ b/include/graphblas/reference/vector.hpp @@ -857,6 +857,24 @@ namespace grb { #endif } + /** + * Creates a reference vector with given values. + * This constructor takes an initializer list of values that will be copied. + */ + Vector( const std::initializer_list< D > vals ) + : Vector( vals.size(), vals.size() ) + { +#ifdef _DEBUG + std::cerr << "In Vector< reference >::Vector( initializer_list )" + << " constructor\n"; +#endif + size_t i = 0; + for( auto each : vals ) { + _raw[ i++ ] = each; + } + _coordinates.assignAll(); + } + /** * The default constructor creates an empty vector and should never be * used explicitly. From be295d0b9e3136ddcb98a1dc6b7ea1db4c6d88c0 Mon Sep 17 00:00:00 2001 From: Benjamin Lozes Date: Wed, 19 Jul 2023 14:09:24 +0200 Subject: [PATCH 04/13] Implementation in nonblocking backend --- include/graphblas/nonblocking/coordinates.hpp | 6 ++++++ include/graphblas/nonblocking/vector.hpp | 18 +++++++++++++++++- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/include/graphblas/nonblocking/coordinates.hpp b/include/graphblas/nonblocking/coordinates.hpp index 592d77ca1..d5d2e26af 100644 --- a/include/graphblas/nonblocking/coordinates.hpp +++ b/include/graphblas/nonblocking/coordinates.hpp @@ -318,6 +318,12 @@ namespace grb { } } + template< bool maybe_invalid = false > + inline void assignAll( ) noexcept { + // Must be defined with the same name as the reference backend + return local_assignAllNotAlreadyAssigned< maybe_invalid >( ); + } + inline void clear() noexcept { if( _n == _cap ) { diff --git a/include/graphblas/nonblocking/vector.hpp b/include/graphblas/nonblocking/vector.hpp index 096b58ef5..2a6269f31 100644 --- a/include/graphblas/nonblocking/vector.hpp +++ b/include/graphblas/nonblocking/vector.hpp @@ -255,7 +255,14 @@ namespace grb { typedef typename Vector< D, reference, MyCoordinates >::const_iterator const_iterator; - Vector( const size_t n, const size_t nz ) : ref( n, nz ) {} + Vector( const size_t n, const size_t nz ) : ref( n, nz ) { + // pipeline execution is not required here as this is a grb::Vector + // declaration +#ifdef _DEBUG + std::cerr << "In Vector< nonblocking >::Vector( size_t, size_t )" + << " constructor\n"; +#endif + } Vector( const size_t n ) : Vector( n, n ) { @@ -266,6 +273,15 @@ namespace grb { #endif } + Vector( const std::initializer_list< D > vals ) : ref( vals ) { + // pipeline execution is not required here as this is a grb::Vector + // declaration +#ifdef _DEBUG + std::cerr << "In Vector< nonblocking >::Vector( initializer_list )" + << " constructor\n"; +#endif + } + Vector() : Vector( 0 ) {} Vector( const Vector< D, nonblocking, MyCoordinates > &x ) : From 2a6529d2139060dfdfd7afe77fd5ff4644efdae8 Mon Sep 17 00:00:00 2001 From: Benjamin Lozes Date: Wed, 19 Jul 2023 14:09:38 +0200 Subject: [PATCH 05/13] Implementation in hyperdags backend --- include/graphblas/hyperdags/io.hpp | 3 ++- include/graphblas/hyperdags/vector.hpp | 8 ++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/include/graphblas/hyperdags/io.hpp b/include/graphblas/hyperdags/io.hpp index db1f09e54..09b16634a 100644 --- a/include/graphblas/hyperdags/io.hpp +++ b/include/graphblas/hyperdags/io.hpp @@ -180,7 +180,8 @@ namespace grb { typename T > RC set( - Vector< DataType, hyperdags, Coords > &x, const T val, + Vector< DataType, hyperdags, Coords > &x, + const T val, const Phase &phase = EXECUTE, const typename std::enable_if< !grb::is_object< DataType >::value && diff --git a/include/graphblas/hyperdags/vector.hpp b/include/graphblas/hyperdags/vector.hpp index 5f422399e..a0aff17a5 100644 --- a/include/graphblas/hyperdags/vector.hpp +++ b/include/graphblas/hyperdags/vector.hpp @@ -161,6 +161,14 @@ namespace grb { register_vector(); } + Vector( const std::initializer_list< T > vals ) : vector( vals ) + { +#ifdef _DEBUG + std::cout << "In Vector< hyperdags >::Vector( initializer_list )" + << " constructor\n"; +#endif + } + ~Vector() { #ifdef _DEBUG std::cout << "Vector (hyperdags) destructor\n"; From 6bcb83aa03be40af7185e7759f677b139c1d84f8 Mon Sep 17 00:00:00 2001 From: Benjamin Lozes Date: Wed, 19 Jul 2023 14:31:50 +0200 Subject: [PATCH 06/13] Implementation in BSP1D backend --- include/graphblas/bsp1d/vector.hpp | 54 ++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/include/graphblas/bsp1d/vector.hpp b/include/graphblas/bsp1d/vector.hpp index 1e85db74e..6d25745ba 100644 --- a/include/graphblas/bsp1d/vector.hpp +++ b/include/graphblas/bsp1d/vector.hpp @@ -2377,6 +2377,60 @@ namespace grb { #endif } + /** + * Creates a dense ALP/GraphBLAS vector. This constructor takes an initializer + * list of values that will be copied into this vector. + */ + Vector( const std::initializer_list< D > &vals ) + : Vector( vals.size(), vals.size() ) + { +#ifdef _DEBUG + std::cerr << "In Vector< BSP1D >::Vector( initializer_list ) constructor\n"; +#endif + RC ret = SUCCESS; + const size_t n = vals.size(); + const internal::BSP1D_Data &data = internal::grb_BSP1D.cload(); + + // Set all the local values + for( size_t i = 0; i < vals.size(); i++ ) { + const D val = *( vals.begin() + i ); + + // check if local + // if( (i / x._b) % data.P != data.s ) { + if( data.s != + internal::Distribution< BSP1D >::global_index_to_process_id( + i, n, data.P + ) + ) { + continue; + } + + // local, so translate index and perform requested operation + const size_t local_index = + internal::Distribution< BSP1D >::global_index_to_local( i, n, data.P ); +#ifdef _DEBUG + std::cout << data.s << ", grb::setElement translates global index " + << i << " to " << local_index << "\n"; +#endif + ret = ret + ? ret + : setElement( + _local, val, local_index, EXECUTE + ); + } + + // Synchronise once between all processes + if( SUCCESS != + collectives< BSP1D >::allreduce( ret, operators::any_or< RC >() ) + ) { + throw std::runtime_error( "grb::Vector< BSP1D >::Vector( initializer_list ): " + "collective::allreduce failed." ); + } + + // on successful execute, sync new nnz count + updateNnz(); + } + /** * Copy constructor. * From d0404a5d5f340ba40ca3be472503d84c751b7ef3 Mon Sep 17 00:00:00 2001 From: Benjamin Lozes Date: Thu, 3 Aug 2023 19:31:12 +0200 Subject: [PATCH 07/13] Review fixes --- include/graphblas/base/vector.hpp | 11 +++++++++-- include/graphblas/bsp1d/vector.hpp | 15 +++++++++------ include/graphblas/hyperdags/vector.hpp | 1 + include/graphblas/nonblocking/coordinates.hpp | 4 ++-- include/graphblas/reference/vector.hpp | 14 +++++++++----- tests/unit/vectorFromListConstructor.cpp | 17 +++++++++-------- 6 files changed, 39 insertions(+), 23 deletions(-) diff --git a/include/graphblas/base/vector.hpp b/include/graphblas/base/vector.hpp index 3aabe66a7..736075f75 100644 --- a/include/graphblas/base/vector.hpp +++ b/include/graphblas/base/vector.hpp @@ -237,8 +237,15 @@ namespace grb { } /** - * Creates a dense ALP/GraphBLAS vector. This constructor takes an initializer - * list of values that will be copied into this vector. + * Creates a dense ALP/GraphBLAS vector. + * This constructor takes an initializer list of values that will be copied + * into this vector. + * The size of the vector will be equal to the number of elements in the + * initializer list. + * + * @param[in] vals The values to be copied into this vector. + * Values will be read in the order in which they are + * supplied. */ Vector( const std::initializer_list< D > &vals ) { (void) vals; diff --git a/include/graphblas/bsp1d/vector.hpp b/include/graphblas/bsp1d/vector.hpp index 6d25745ba..23c0ed4d4 100644 --- a/include/graphblas/bsp1d/vector.hpp +++ b/include/graphblas/bsp1d/vector.hpp @@ -2378,8 +2378,13 @@ namespace grb { } /** - * Creates a dense ALP/GraphBLAS vector. This constructor takes an initializer - * list of values that will be copied into this vector. + * Constructs a BSP1D vector. + * + * @see Full description in base backend. + * + * \internal + * This routine initialises the local vector and synchronises the global + * vector once at the end. */ Vector( const std::initializer_list< D > &vals ) : Vector( vals.size(), vals.size() ) @@ -2413,10 +2418,8 @@ namespace grb { << i << " to " << local_index << "\n"; #endif ret = ret - ? ret - : setElement( - _local, val, local_index, EXECUTE - ); + ? ret + : setElement( _local, val, local_index, EXECUTE ); } // Synchronise once between all processes diff --git a/include/graphblas/hyperdags/vector.hpp b/include/graphblas/hyperdags/vector.hpp index a0aff17a5..02c0f7734 100644 --- a/include/graphblas/hyperdags/vector.hpp +++ b/include/graphblas/hyperdags/vector.hpp @@ -167,6 +167,7 @@ namespace grb { std::cout << "In Vector< hyperdags >::Vector( initializer_list )" << " constructor\n"; #endif + register_vector(); } ~Vector() { diff --git a/include/graphblas/nonblocking/coordinates.hpp b/include/graphblas/nonblocking/coordinates.hpp index d5d2e26af..ae0eecc3f 100644 --- a/include/graphblas/nonblocking/coordinates.hpp +++ b/include/graphblas/nonblocking/coordinates.hpp @@ -265,7 +265,7 @@ namespace grb { } template< bool maybe_invalid = false > - inline void local_assignAll( ) noexcept { + inline void local_assignAll() noexcept { if( maybe_invalid || _n != _cap ) { if( _assigned != nullptr ) { assert( _stack != nullptr ); @@ -319,7 +319,7 @@ namespace grb { } template< bool maybe_invalid = false > - inline void assignAll( ) noexcept { + inline void assignAll() noexcept { // Must be defined with the same name as the reference backend return local_assignAllNotAlreadyAssigned< maybe_invalid >( ); } diff --git a/include/graphblas/reference/vector.hpp b/include/graphblas/reference/vector.hpp index dd016bab6..5175453cc 100644 --- a/include/graphblas/reference/vector.hpp +++ b/include/graphblas/reference/vector.hpp @@ -858,8 +858,9 @@ namespace grb { } /** - * Creates a reference vector with given values. - * This constructor takes an initializer list of values that will be copied. + * Constructs a reference vector. + * + * @see Full description in base backend. */ Vector( const std::initializer_list< D > vals ) : Vector( vals.size(), vals.size() ) @@ -868,9 +869,12 @@ namespace grb { std::cerr << "In Vector< reference >::Vector( initializer_list )" << " constructor\n"; #endif - size_t i = 0; - for( auto each : vals ) { - _raw[ i++ ] = each; + +#ifdef _H_GRB_REFERENCE_OMP_VECTOR + #pragma omp parallel for simd +#endif + for( size_t i = 0; i < vals.size(); ++i ) { + _raw[ i ] = *( vals.begin() + i ); } _coordinates.assignAll(); } diff --git a/tests/unit/vectorFromListConstructor.cpp b/tests/unit/vectorFromListConstructor.cpp index 6dc55ce96..b2b255ba1 100644 --- a/tests/unit/vectorFromListConstructor.cpp +++ b/tests/unit/vectorFromListConstructor.cpp @@ -20,11 +20,10 @@ #include "graphblas.hpp" -using namespace grb; -void grbProgram( const void *, const size_t in_size, int & error ) { - error = 0; +using namespace grb; +void grbProgram( const void *, const size_t in_size, int &error ) { if( in_size != 0 ) { std::cerr << "Unit tests called with unexpected input" << std::endl; error = 1; @@ -38,7 +37,8 @@ void grbProgram( const void *, const size_t in_size, int & error ) { auto position = vector_it->first; auto value = vector_it->second; if( i != position || values[ i ] != value ) { - std::cerr << "Expected position " << i << " value " << values[ i ] << " but got position " << position << " value " << value << std::endl; + std::cerr << "Expected position " << i << " value " << values[ i ] + << " but got position " << position << " value " << value << std::endl; equals = false; break; } @@ -52,19 +52,20 @@ void grbProgram( const void *, const size_t in_size, int & error ) { } int main( int argc, char ** argv ) { - (void)argc; + (void) argc; std::cout << "Functional test executable: " << argv[ 0 ] << std::endl; - int error; + int error = 0; grb::Launcher< AUTOMATIC > launcher; if( launcher.exec( &grbProgram, NULL, 0, error ) != SUCCESS ) { std::cout << "Test FAILED (test failed to launch)" << std::endl; error = 255; } + std::cerr << std::flush; if( error == 0 ) { - std::cout << "Test OK" << std::endl; + std::cout << std::flush << "Test OK" << std::endl; } else { - std::cout << "Test FAILED" << std::endl; + std::cout << std::flush << "Test FAILED" << std::endl; } // done From 2ade5e277665ac9d591c0ab083300500722fbc19 Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Mon, 2 Oct 2023 13:27:28 +0200 Subject: [PATCH 08/13] Code review, found one TODO / dependence on MR 233 to be resolved first --- include/graphblas/base/vector.hpp | 19 +++++++++++++------ include/graphblas/bsp1d/vector.hpp | 8 ++++++-- include/graphblas/reference/vector.hpp | 2 +- tests/unit/vectorFromListConstructor.cpp | 12 +++++++++--- 4 files changed, 29 insertions(+), 12 deletions(-) diff --git a/include/graphblas/base/vector.hpp b/include/graphblas/base/vector.hpp index 736075f75..624fd4be7 100644 --- a/include/graphblas/base/vector.hpp +++ b/include/graphblas/base/vector.hpp @@ -238,14 +238,21 @@ namespace grb { /** * Creates a dense ALP/GraphBLAS vector. - * This constructor takes an initializer list of values that will be copied - * into this vector. - * The size of the vector will be equal to the number of elements in the - * initializer list. + * + * This constructor takes an initialiser list of values that will be copied + * into this vector. The size of the vector will be equal to the number of + * elements in the initialiser list. + * + * For backends with more than one user process, the size of \a vals is the + * global vector size, and the contents of \a vals are processed using + * sequential I/O semantics. + * + * @see #grb::IOMode For the difference between sequential and parallel I/O + * modes. + * + * \note There is only a difference if there are more than one user process. * * @param[in] vals The values to be copied into this vector. - * Values will be read in the order in which they are - * supplied. */ Vector( const std::initializer_list< D > &vals ) { (void) vals; diff --git a/include/graphblas/bsp1d/vector.hpp b/include/graphblas/bsp1d/vector.hpp index 23c0ed4d4..a0d44829f 100644 --- a/include/graphblas/bsp1d/vector.hpp +++ b/include/graphblas/bsp1d/vector.hpp @@ -2383,8 +2383,12 @@ namespace grb { * @see Full description in base backend. * * \internal - * This routine initialises the local vector and synchronises the global - * vector once at the end. + * This constructor initialises the local vector and synchronises the global + * vector once. + * + * TODO rewrite below logic using an iterator filter (GitHub PR 233, issue + * 228) + * \endinternal */ Vector( const std::initializer_list< D > &vals ) : Vector( vals.size(), vals.size() ) diff --git a/include/graphblas/reference/vector.hpp b/include/graphblas/reference/vector.hpp index 5175453cc..fcd2516d8 100644 --- a/include/graphblas/reference/vector.hpp +++ b/include/graphblas/reference/vector.hpp @@ -862,7 +862,7 @@ namespace grb { * * @see Full description in base backend. */ - Vector( const std::initializer_list< D > vals ) + Vector( const std::initializer_list< D > &vals ) : Vector( vals.size(), vals.size() ) { #ifdef _DEBUG diff --git a/tests/unit/vectorFromListConstructor.cpp b/tests/unit/vectorFromListConstructor.cpp index b2b255ba1..da42e85e6 100644 --- a/tests/unit/vectorFromListConstructor.cpp +++ b/tests/unit/vectorFromListConstructor.cpp @@ -34,19 +34,25 @@ void grbProgram( const void *, const size_t in_size, int &error ) { bool equals = true; auto vector_it = x.begin(); for( size_t i = 0; i < values.size(); i++ ) { + if( vector_it == x.end() ) { + std::cerr << "Vector iterator prematurely in end position!" << std::endl; + error = 10; + return; + } auto position = vector_it->first; auto value = vector_it->second; if( i != position || values[ i ] != value ) { std::cerr << "Expected position " << i << " value " << values[ i ] - << " but got position " << position << " value " << value << std::endl; + << " but got position " << position << " value " << value + << std::endl; equals = false; break; } - vector_it.operator++(); + (void) vector_it.operator++(); } if( !equals ) { std::cerr << "Vector values are not correct" << std::endl; - error = 1; + error = 20; return; } } From 366e8b17ad5b4d18342946e8bdadf6ad24af4227 Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Wed, 7 Feb 2024 17:53:19 +0100 Subject: [PATCH 09/13] Small typo fix --- include/graphblas/base/vector.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/graphblas/base/vector.hpp b/include/graphblas/base/vector.hpp index 624fd4be7..5eb4ad83a 100644 --- a/include/graphblas/base/vector.hpp +++ b/include/graphblas/base/vector.hpp @@ -248,7 +248,7 @@ namespace grb { * sequential I/O semantics. * * @see #grb::IOMode For the difference between sequential and parallel I/O - * modes. + * modes. * * \note There is only a difference if there are more than one user process. * From 967dc5ea22ae6a17c337d3339d69fa4de35bf876 Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Wed, 7 Feb 2024 18:12:38 +0100 Subject: [PATCH 10/13] Small typo fix --- include/graphblas/nonblocking/vector.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/graphblas/nonblocking/vector.hpp b/include/graphblas/nonblocking/vector.hpp index 2a6269f31..a70c48521 100644 --- a/include/graphblas/nonblocking/vector.hpp +++ b/include/graphblas/nonblocking/vector.hpp @@ -273,7 +273,7 @@ namespace grb { #endif } - Vector( const std::initializer_list< D > vals ) : ref( vals ) { + Vector( const std::initializer_list< D > vals ) : ref( vals ) { // pipeline execution is not required here as this is a grb::Vector // declaration #ifdef _DEBUG From 754671bb9bca32a387b0d080f81230c78417fe87 Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Thu, 8 Feb 2024 12:29:43 +0100 Subject: [PATCH 11/13] Minor unrelated warning suppressions --- tests/unit/eWiseLambda.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/unit/eWiseLambda.cpp b/tests/unit/eWiseLambda.cpp index 48ec65088..ef98ffaca 100644 --- a/tests/unit/eWiseLambda.cpp +++ b/tests/unit/eWiseLambda.cpp @@ -49,7 +49,10 @@ void grbProgram( const void *, const size_t in_size, int &error ) { } if( !error ) { - rc = grb::eWiseLambda( [&M]( const size_t i, const size_t j, int& nz ) { nz = j; }, M ); + rc = grb::eWiseLambda( [&M]( const size_t i, const size_t j, int& nz ) { + (void) i; + nz = j; + }, M ); } if( rc != SUCCESS ) { std::cerr << "\t eWiseLambda call failed\n"; @@ -57,8 +60,8 @@ void grbProgram( const void *, const size_t in_size, int &error ) { } if( !error ) { - for(auto it: M) { - if( it.second != it.first.second ) { + for( const auto &it : M ) { + if( static_cast< size_t >(it.second) != it.first.second ) { std::cerr << "\t eWiseLambda returned incorrect result\n"; error = 15; break; From c54f8216050a170ffd921ab8c8ef8ac0adaeb183 Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Thu, 8 Feb 2024 12:38:47 +0100 Subject: [PATCH 12/13] assignAll should call local_assignAll, and local_assignAll should be parallelised --- include/graphblas/nonblocking/coordinates.hpp | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/include/graphblas/nonblocking/coordinates.hpp b/include/graphblas/nonblocking/coordinates.hpp index ae0eecc3f..58371cd3b 100644 --- a/include/graphblas/nonblocking/coordinates.hpp +++ b/include/graphblas/nonblocking/coordinates.hpp @@ -273,9 +273,14 @@ namespace grb { assert( !maybe_invalid || _n <= _cap ); _n = _cap; - for( size_t i = 0; i < _n; ++i ) { - _assigned[ i ] = true; - _stack[ i ] = i; + #pragma omp parallel + { + size_t start, end; + config::OMP::localRange( start, end, 0, _n ); + for( size_t i = start; i < end; ++i ) { + _assigned[ i ] = true; + _stack[ i ] = i; + } } } } @@ -321,7 +326,7 @@ namespace grb { template< bool maybe_invalid = false > inline void assignAll() noexcept { // Must be defined with the same name as the reference backend - return local_assignAllNotAlreadyAssigned< maybe_invalid >( ); + return local_assignAll< maybe_invalid >(); } inline void clear() noexcept { From 788858be858f6aff9d6a614484677683d9815abe Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Thu, 8 Feb 2024 13:13:46 +0100 Subject: [PATCH 13/13] Revert earlier (and incorrect) change, meaning the assignedAll must have its own dedicated implementation after all, herewith provided --- include/graphblas/nonblocking/coordinates.hpp | 27 ++++++++++--------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/include/graphblas/nonblocking/coordinates.hpp b/include/graphblas/nonblocking/coordinates.hpp index 58371cd3b..ad9e3c670 100644 --- a/include/graphblas/nonblocking/coordinates.hpp +++ b/include/graphblas/nonblocking/coordinates.hpp @@ -273,14 +273,9 @@ namespace grb { assert( !maybe_invalid || _n <= _cap ); _n = _cap; - #pragma omp parallel - { - size_t start, end; - config::OMP::localRange( start, end, 0, _n ); - for( size_t i = start; i < end; ++i ) { - _assigned[ i ] = true; - _stack[ i ] = i; - } + for( size_t i = 0; i < _n; ++i ) { + _assigned[ i ] = true; + _stack[ i ] = i; } } } @@ -323,14 +318,21 @@ namespace grb { } } - template< bool maybe_invalid = false > inline void assignAll() noexcept { - // Must be defined with the same name as the reference backend - return local_assignAll< maybe_invalid >(); + // this operates on the global coordinates, not on a local view of it + #pragma omp parallel + { + size_t start, end; + config::OMP::localRange( start, end, 0, _cap ); + for( size_t i = start; i < end; ++i ) { + _assigned[ i ] = true; + _stack[ i ] = i; + } + } + _n = _cap; } inline void clear() noexcept { - if( _n == _cap ) { #ifndef NDEBUG if( _assigned == nullptr && _cap > 0 ) { @@ -338,7 +340,6 @@ namespace grb { assert( dense_coordinates_may_not_call_clear ); } #endif - #pragma omp parallel for schedule( dynamic, config::CACHE_LINE_SIZE::value() ) for( size_t i = 0; i < _cap; ++i ) { _assigned[ i ] = false;