Skip to content

Commit

Permalink
Review fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
byjtew committed Feb 5, 2024
1 parent 8f9a28b commit c63b55c
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 26 deletions.
4 changes: 2 additions & 2 deletions include/graphblas/base/io.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
47 changes: 31 additions & 16 deletions include/graphblas/reference/compressed_storage.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@

#include <cstring> //std::memcpy

#if reference == reference_omp
#define _H_GRB_REFERENCE_OMP_COMPRESSED_STORAGE
#include <omp.h>
#endif


namespace grb {

Expand Down Expand Up @@ -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 );
Expand All @@ -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 ] );
Expand Down Expand Up @@ -585,13 +592,16 @@ namespace grb {
*
* Performs no safety checking. Performs no (re-)allocations.
*
* @tparam use_id If set to <tt>true</tt>, 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 <tt>true</tt>.
* The copy range is 2nz + m + 1, i.e.,
* -# 0 <= start < 2nz + m + 1
* -# 0 < end <= 2nz + m + 1
Expand Down Expand Up @@ -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;
Expand All @@ -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.
Expand Down Expand Up @@ -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";
Expand Down
15 changes: 12 additions & 3 deletions include/graphblas/reference/io.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 ) ),
Expand All @@ -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 );
Expand Down
7 changes: 2 additions & 5 deletions tests/unit/copyMixedDomainsMatrices.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@

#include <graphblas.hpp>


using namespace grb;


Expand Down Expand Up @@ -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 );
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit c63b55c

Please sign in to comment.