From c63b55c41633b60e33418d1401d6ecfe7e709b2c Mon Sep 17 00:00:00 2001 From: Benjamin Lozes Date: Mon, 5 Feb 2024 16:04:14 +0100 Subject: [PATCH] Review fixes --- include/graphblas/base/io.hpp | 4 +- .../reference/compressed_storage.hpp | 47 ++++++++++++------- include/graphblas/reference/io.hpp | 15 ++++-- tests/unit/copyMixedDomainsMatrices.cpp | 7 +-- 4 files changed, 47 insertions(+), 26 deletions(-) diff --git a/include/graphblas/base/io.hpp b/include/graphblas/base/io.hpp index 743a1c4b4..9341636e6 100644 --- a/include/graphblas/base/io.hpp +++ b/include/graphblas/base/io.hpp @@ -1123,8 +1123,8 @@ namespace grb { const Phase &phase = EXECUTE ) noexcept { #ifndef NDEBUG - const bool should_not_call_base_matrix_set_copy_masked = false; - assert( should_not_call_base_matrix_set_copy_masked ); + const bool should_not_call_base_matrix_set_copy = false; + assert( should_not_call_base_matrix_set_copy ); #endif (void) A; (void) C; diff --git a/include/graphblas/reference/compressed_storage.hpp b/include/graphblas/reference/compressed_storage.hpp index fcdcf6a3e..356843223 100644 --- a/include/graphblas/reference/compressed_storage.hpp +++ b/include/graphblas/reference/compressed_storage.hpp @@ -25,6 +25,11 @@ #include //std::memcpy +#if reference == reference_omp + #define _H_GRB_REFERENCE_OMP_COMPRESSED_STORAGE + #include +#endif + namespace grb { @@ -76,11 +81,12 @@ namespace grb { if( k < nz ) { const size_t loop_end = std::min( nz, end ); assert( k <= loop_end ); - std::copy_n( - input.row_index + k, - loop_end - k, - output.row_index + k - ); +#ifdef _H_GRB_REFERENCE_OMP_COMPRESSED_STORAGE + #pragma omp for simd +#endif + for( size_t i = k; i < loop_end; ++i ) { + output.row_index[ i ] = static_cast< OutputIND >( input.row_index[ i ] ); + } k = 0; } else { assert( k >= nz ); @@ -93,11 +99,12 @@ namespace grb { if( k < m + 1 ) { const size_t loop_end = std::min( m + 1, end ); assert( k <= loop_end ); - std::copy_n( - input.col_start + k, - loop_end - k, - output.col_start + k - ); +#ifdef _H_GRB_REFERENCE_OMP_COMPRESSED_STORAGE + #pragma omp for simd +#endif + for( size_t i = k; i < loop_end; ++i ) { + output.col_start[ i ] = static_cast< OutputSIZE >( input.col_start[ i ] ); + } #ifndef NDEBUG for( size_t chk = k; chk < loop_end - 1; ++chk ) { assert( input.col_start[ chk ] <= input.col_start[ chk + 1 ] ); @@ -585,13 +592,16 @@ namespace grb { * * Performs no safety checking. Performs no (re-)allocations. * + * @tparam use_id If set to true, use \a id instead of values in + * \a other. * @param[in] other The container to copy from. * @param[in] nz The number of nonzeroes in the \a other container. * @param[in] m The index dimension of the \a other container. * @param[in] start The start position to copy from (inclusive). * @param[in] end The end position to copy to (exclusive). * @param[in] id A pointer to a value overriding those in \a other. - * + * Will only be used if and only if \a use_id is set + * to true. * The copy range is 2nz + m + 1, i.e., * -# 0 <= start < 2nz + m + 1 * -# 0 < end <= 2nz + m + 1 @@ -680,7 +690,12 @@ namespace grb { assert( k <= loop_end ); GRB_UTIL_IGNORE_CLASS_MEMACCESS; - std::copy_n( other.values + k, loop_end - k, values + k ); +#ifdef _H_GRB_REFERENCE_OMP_COMPRESSED_STORAGE + #pragma omp for simd +#endif + for( size_t i = k; i < loop_end; ++i ) { + values[ i ] = static_cast< D >( other.values[ i ] ); + } GRB_UTIL_RESTORE_WARNINGS; k = 0; @@ -696,8 +711,6 @@ namespace grb { copyCoordinatesFrom( *this, other, nz, m, k, end ); } - - /** * Writes a nonzero to the given position. Does \em not update the * \a col_start array. Does not perform any type checking. @@ -1203,8 +1216,10 @@ namespace grb { const size_t nz, const size_t m, const size_t start, size_t end, const UnusedType * __restrict__ = nullptr ) { - // the use_id template is meaningless in the case of pattern matrices, but - // is retained to keep the API the same as with the non-pattern case. + (void) unusedValue; + // the unusedValue template is meaningless in the case of + // pattern matrices, but is retained to keep the API + // the same as with the non-pattern case. #ifdef _DEBUG std::cout << "CompressedStorage::copyFrom (void) called with range " << start << "--" << end << "\n"; diff --git a/include/graphblas/reference/io.hpp b/include/graphblas/reference/io.hpp index e1ca010ea..3ee301fd1 100644 --- a/include/graphblas/reference/io.hpp +++ b/include/graphblas/reference/io.hpp @@ -982,15 +982,18 @@ namespace grb { #ifdef _DEBUG std::cout << "Called grb::set (matrices, reference), execute phase\n"; #endif - if( A_is_mask ) { - assert( id != nullptr ); - } // static checks static_assert( !A_is_mask || !std::is_void< OutputType >::value, "grb::set (matrices, reference): " "if A is a mask, then C must not be a void matrix" ); + static_assert( + A_is_mask || + ( !A_is_mask && + std::is_convertible< ValueType, OutputType >::value ), + "internal::grb::set called with non-matching value types" + ); NO_CAST_ASSERT( ( !( descr & descriptors::no_casting ) || ( !A_is_mask && std::is_same< InputType, OutputType >::value ) ), @@ -1016,6 +1019,12 @@ namespace grb { "internal::grb::set", "Called with non-matching value types" ); +#ifndef NDEBUG + if( A_is_mask ) { + assert( id != nullptr ); + } +#endif + // run-time checks const size_t m = nrows( A ); const size_t n = ncols( A ); diff --git a/tests/unit/copyMixedDomainsMatrices.cpp b/tests/unit/copyMixedDomainsMatrices.cpp index a5c3d05ad..290223045 100644 --- a/tests/unit/copyMixedDomainsMatrices.cpp +++ b/tests/unit/copyMixedDomainsMatrices.cpp @@ -22,7 +22,6 @@ #include - using namespace grb; @@ -54,8 +53,6 @@ void grb_program( const size_t &n, grb::RC &rc ) { } } - - { // Try cast to ushort (should succeed) Matrix< ushort > M_short( n, n, 0UL ); rc = grb::set( M_short, A_int, Phase::RESIZE ); @@ -176,10 +173,10 @@ int main( int argc, char ** argv ) { if( argc == 2 ) { size_t read; std::istringstream ss( argv[ 1 ] ); - if( ! ( ss >> read ) ) { + if( !( ss >> read ) ) { std::cerr << "Error parsing first argument\n"; printUsage = true; - } else if( ! ss.eof() ) { + } else if( !ss.eof() ) { std::cerr << "Error parsing first argument\n"; printUsage = true; } else {