Skip to content

Commit

Permalink
632 Matrix eWiseApply implemented but not declared in base (#184)
Browse files Browse the repository at this point in the history 
Several eWiseApply primitives for matrix inputs and outputs were implemented within all backends, but lacked a base specification. This MR adds a base specification of the operator *and* monoid based matrix eWiseApply to `include/graphblas/base/blas3.hpp`. In providing such, it found and fixes the following items:

- matrix eWiseApply did not compile for non-standard matrix index types;
- some other primitives did not compile for non-standard matrix types (e.g., `zip`); while
- yet other primitives assumed uniform matrix index types for all matrix arguments (e.g., `mxm`).

Different backends had several primitives for which this occurred; see this MR details for information on which primitives were affected. This MR included a code review that fixes all such occurrences in *all* backends except banshee--see note [1]. This MR does *not* resolve a previously flagged issue about the testing of matrix-based eWiseApply variants-- see note [2].

Additionally, this MR fixes several issues where passing the algebraic structure as a template argument would not work due to erroneously ordered template arguments. For example, both bottom lines in the following snippet should be equivalent:

```
typedef Semriring< operators::add< double >, operators::mul< double >, identities::zero, identities::one > MyRing;
MyRing ring;
grb::mxv< descriptors::no_operation >( y, A, x, ring );
grb::mxv< descriptors::no_operation, MyRing >( y, A, x );
```

The latter variant requires the algebraic structure types to follow the descriptor template argument. This MR fixes several occurrences where this was not the case. (Though both variants are supported, the former is the suggested usage.)

Finally, this MR applies various minor code style fixes throughout all files it touches.

[1] In the regular usage of ALP/GraphBLAS, the issues regarding custom matrix index types that this MR solves, do not trigger compile-time nor run-time (functional) bugs. The use or compilation of the SparseBLAS and SpBLAS transition APIs (the latter of which does employ different matrix index types) likewise does not trigger related compilation or run-time issues. This MR however notably does *not* include extensive testing for primitives with multiple matrix arguments that use differing index types-- see GitHub issue #205.

[2] In resolving this MR it was detected that unit testing only was done for the monoid-based matrix eWiseApply, not for the operator-based variant. This MR solves the issue that for new backends, code calling an unimplemented operator-based variant now does correctly return `UNSUPPORTED` (and triggering an assertion failure when in debug mode). However, it does not enable the automatic detection of such a missing implementation during standard unit testing-- this is to be resolved as part of GitHub issue #203 .
  • Loading branch information
@byjtew
byjtew authored Jun 22, 2023
1 parent 84ca6d1 commit 3b5e72f
Show file tree
Hide file tree
Showing 11 changed files with 525 additions and 254 deletions.
208 changes: 204 additions & 4 deletions include/graphblas/base/blas3.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,14 +87,16 @@ namespace grb {
template<
Descriptor descr = descriptors::no_operation,
typename OutputType, typename InputType1, typename InputType2,
typename CIT, typename RIT, typename NIT,
typename CIT1, typename RIT1, typename NIT1,
typename CIT2, typename RIT2, typename NIT2,
typename CIT3, typename RIT3, typename NIT3,
class Semiring,
Backend backend
>
RC mxm(
Matrix< OutputType, backend, CIT, RIT, NIT > &C,
const Matrix< InputType1, backend, CIT, RIT, NIT > &A,
const Matrix< InputType2, backend, CIT, RIT, NIT > &B,
Matrix< OutputType, backend, CIT1, RIT1, NIT1 > &C,
const Matrix< InputType1, backend, CIT2, RIT2, NIT2 > &A,
const Matrix< InputType2, backend, CIT3, RIT3, NIT3 > &B,
const Semiring &ring = Semiring(),
const Phase &phase = EXECUTE
) {
Expand Down Expand Up @@ -242,6 +244,204 @@ namespace grb {
return ret == SUCCESS ? UNSUPPORTED : ret;
}

/**
* Computes \f$ C = A \odot B \f$, out of place, monoid variant.
*
* Calculates the element-wise operation on one scalar to elements of one
* matrix, \f$ C = A \odot B \f$, using the given monoid's operator. The input
* and output matrices must be of same dimension.
*
* Any old entries of \a C will be removed after a successful call to this
* primitive; that is, this is an out-of-place primitive.
*
* After a successful call to this primitive, the nonzero structure of \a C
* will match that of the union of \a A and \a B. An implementing backend may
* skip processing rows \a i and columns \a j that are not in the union of the
* nonzero structure of \a A and \a B.
*
* \note When applying element-wise operators on sparse matrices using
* semirings, there is a difference between interpreting missing
* values as an annihilating identity or as a neutral identity--
* intuitively, such identities are known as `zero' or `one',
* respectively. As a consequence, this functionality is provided by
* #grb::eWiseApply depending on whether a monoid or operator is
* provided:
* - #grb::eWiseApply using monoids (neutral),
* - #grb::eWiseApply using operators (annihilating).
*
* @tparam descr The descriptor to be used. Optional; the default is
* #grb::descriptors::no_operation.
* @tparam Monoid The monoid to use.
* @tparam InputType1 The value type of the left-hand matrix.
* @tparam InputType2 The value type of the right-hand matrix.
* @tparam OutputType The value type of the ouput matrix.
*
* @param[out] C The output matrix.
* @param[in] A The left-hand input matrix.
* @param[in] B The right-hand input matrix.
* @param[in] monoid The monoid structure containing \f$ \odot \f$.
* @param[in] phase The #grb::Phase the call should execute. Optional; the
* default parameter is #grb::EXECUTE.
*
* @return #grb::SUCCESS On successful completion of this call.
* @return #grb::MISMATCH Whenever the dimensions of \a x, \a y and \a z do
* not match. All input data containers are left
* untouched if this exit code is returned; it will be
* as though this call was never made.
* @return #grb::FAILED If \a phase is #grb::EXECUTE, indicates that the
* capacity of \a z was insufficient. The output matrix
* \a z is cleared, and the call to this function has
* no further effects.
* @return #grb::OUTOFMEM If \a phase is #grb::RESIZE, indicates an
* out-of-memory exception. The call to this function
* shall have no other effects beyond *returning this
* error code; the previous state of \a z is retained.
* @return #grb::PANIC A general unmitigable error has been encountered. If
* returned, ALP enters an undefined state and the user
* program is encouraged to exit as quickly as possible.
*
* \par Performance semantics
*
* Each backend must define performance semantics for this primitive.
*
* @see perfSemantics
*/
template<
Descriptor descr = descriptors::no_operation,
class Monoid,
typename OutputType, typename InputType1, typename InputType2,
typename RIT1, typename CIT1, typename NIT1,
typename RIT2, typename CIT2, typename NIT2,
typename RIT3, typename CIT3, typename NIT3,
Backend backend
>
RC eWiseApply(
Matrix< OutputType, backend, RIT1, CIT1, NIT1 > &C,
const Matrix< InputType1, backend, RIT2, CIT2, NIT2 > &A,
const Matrix< InputType2, backend, RIT3, CIT3, NIT3 > &B,
const Monoid &monoid,
const Phase phase = EXECUTE,
const typename std::enable_if<
!grb::is_object< OutputType >::value &&
!grb::is_object< InputType1 >::value &&
!grb::is_object< InputType2 >::value &&
grb::is_monoid< Monoid >::value,
void >::type * const = nullptr
) {
(void) C;
(void) A;
(void) B;
(void) phase;
#ifdef _DEBUG
std::cerr << "Selected backend does not implement grb::eWiseApply\n";
#endif
#ifndef NDEBUG
const bool selected_backend_does_not_support_ewiseapply = false;
assert( selected_backend_does_not_support_ewiseapply );
#endif
const RC ret = grb::clear( A );
return ret == SUCCESS ? UNSUPPORTED : ret;
}

/**
* Computes \f$ C = A \odot B \f$, out of place, operator variant.
*
* Calculates the element-wise operation on one scalar to elements of one
* matrix, \f$ C = A \odot B \f$, using the given operator. The input and
* output matrices must be of same dimension.
*
* Any old entries of \a C will be removed after a successful call to this
* primitive; that is, this primitive is out-of-place.
*
* After a successful call to this primitive, the nonzero structure of \a C
* will match that of the intersection of \a A and \a B. An implementing
* backend may skip processing rows \a i and columns \a j that are not in the
* intersection of the nonzero structure of \a A and \a B.
*
* \note When applying element-wise operators on sparse matrices using
* semirings, there is a difference between interpreting missing
* values as an annihilating identity or as a neutral identity--
* intuitively, such identities are known as `zero' or `one',
* respectively. As a consequence, this functionality is provided by
* #grb::eWiseApply depending on whether a monoid or operator is
* provided:
* - #grb::eWiseApply using monoids (neutral),
* - #grb::eWiseApply using operators (annihilating).
*
* @tparam descr The descriptor to be used. Optional; the default is
* #grb::descriptors::no_operation.
* @tparam Operator The operator to use.
* @tparam InputType1 The value type of the left-hand matrix.
* @tparam InputType2 The value type of the right-hand matrix.
* @tparam OutputType The value type of the ouput matrix.
*
* @param[out] C The output matrix.
* @param[in] A The left-hand input matrix.
* @param[in] B The right-hand input matrix.
* @param[in] op The operator.
* @param[in] phase The #grb::Phase the call should execute. Optional; the
* default parameter is #grb::EXECUTE.
*
* @return #grb::SUCCESS On successful completion of this call.
* @return #grb::MISMATCH Whenever the dimensions of \a x, \a y and \a z do
* not match. All input data containers are left
* untouched if this exit code is returned; it will be
* be as though this call was never made.
* @return #grb::FAILED If \a phase is #grb::EXECUTE, indicates that the
* capacity of \a z was insufficient. The output
* matrix \a z is cleared, and the call to this function
* has no further effects.
* @return #grb::OUTOFMEM If \a phase is #grb::RESIZE, indicates an
* out-of-memory exception. The call to this function
* shall have no other effects beyond returning this
* error code; the previous state of \a z is retained.
* @return #grb::PANIC A general unmitigable error has been encountered. If
* returned, ALP enters an undefined state and the user
* program is encouraged to exit as quickly as possible.
*
* \par Performance semantics
*
* Each backend must define performance semantics for this primitive.
*
* @see perfSemantics
*/
template<
Descriptor descr = grb::descriptors::no_operation,
class Operator,
typename OutputType, typename InputType1, typename InputType2,
typename RIT1, typename CIT1, typename NIT1,
typename RIT2, typename CIT2, typename NIT2,
typename RIT3, typename CIT3, typename NIT3,
Backend backend
>
RC eWiseApply(
Matrix< OutputType, backend, RIT1, CIT1, NIT1 > &C,
const Matrix< InputType1, backend, RIT2, CIT2, NIT2 > &A,
const Matrix< InputType2, backend, RIT3, CIT3, NIT3 > &B,
const Operator &op,
const Phase phase = EXECUTE,
const typename std::enable_if<
!grb::is_object< OutputType >::value &&
!grb::is_object< InputType1 >::value &&
!grb::is_object< InputType2 >::value &&
grb::is_operator< Operator >::value,
void >::type * const = nullptr
) {
(void) C;
(void) A;
(void) B;
(void) phase;
#ifdef _DEBUG
std::cerr << "Selected backend does not implement grb::eWiseApply\n";
#endif
#ifndef NDEBUG
const bool selected_backend_does_not_support_ewiseapply = false;
assert( selected_backend_does_not_support_ewiseapply );
#endif
const RC ret = grb::clear( A );
return ret == SUCCESS ? UNSUPPORTED : ret;
}

/**
* @}
*/
Expand Down
11 changes: 6 additions & 5 deletions include/graphblas/base/io.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -784,7 +784,8 @@ namespace grb {
typename RIT, typename CIT, typename NIT
>
RC resize(
Matrix< InputType, backend, RIT, CIT, NIT > &A, const size_t new_nz
Matrix< InputType, backend, RIT, CIT, NIT > &A,
const size_t new_nz
) noexcept {
#ifndef NDEBUG
const bool should_not_call_base_matrix_resize = false;
Expand Down Expand Up @@ -1327,14 +1328,14 @@ namespace grb {
*/
template<
Descriptor descr = descriptors::no_operation,
typename InputType,
typename InputType, typename RIT, typename CIT, typename NIT,
typename fwd_iterator1 = const size_t * __restrict__,
typename fwd_iterator2 = const size_t * __restrict__,
typename fwd_iterator3 = const InputType * __restrict__,
Backend implementation = config::default_backend
>
RC buildMatrixUnique(
Matrix< InputType, implementation > &A,
Matrix< InputType, implementation, RIT, CIT, NIT > &A,
fwd_iterator1 I, const fwd_iterator1 I_end,
fwd_iterator2 J, const fwd_iterator2 J_end,
fwd_iterator3 V, const fwd_iterator3 V_end,
Expand All @@ -1359,14 +1360,14 @@ namespace grb {
*/
template<
Descriptor descr = descriptors::no_operation,
typename InputType,
typename InputType, typename RIT, typename CIT, typename NIT,
typename fwd_iterator1 = const size_t * __restrict__,
typename fwd_iterator2 = const size_t * __restrict__,
typename fwd_iterator3 = const InputType * __restrict__,
Backend implementation = config::default_backend
>
RC buildMatrixUnique(
Matrix< InputType, implementation > &A,
Matrix< InputType, implementation, RIT, CIT, NIT > &A,
fwd_iterator1 I, fwd_iterator2 J, fwd_iterator3 V,
const size_t nz, const IOMode mode
) {
Expand Down
3 changes: 2 additions & 1 deletion include/graphblas/bsp1d/blas2.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -449,12 +449,13 @@ namespace grb {
typename InputType2 = typename Ring::D2,
typename InputType3 = bool,
typename InputType4 = bool,
typename RIT, typename CIT, typename NIT,
typename Coords
>
RC mxv(
Vector< IOType, BSP1D, Coords > &u,
const Vector< InputType3, BSP1D, Coords > &mask,
const Matrix< InputType2, BSP1D > &A,
const Matrix< InputType2, BSP1D, RIT, CIT, NIT > &A,
const Vector< InputType1, BSP1D, Coords > &v,
const Ring &ring = Ring(),
const Phase &phase = EXECUTE,
Expand Down
61 changes: 40 additions & 21 deletions include/graphblas/bsp1d/blas3.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,12 @@ namespace grb {
* cleared.
* \endinternal
*/
template< typename DataType, Backend backend >
template<
typename DataType, Backend backend,
typename RIT, typename CIT, typename NIT
>
RC checkGlobalErrorStateOrClear(
Matrix< DataType, backend > &A,
Matrix< DataType, backend, RIT, CIT, NIT > &A,
const RC local_rc
) noexcept {
RC global_rc = local_rc;
Expand All @@ -67,11 +70,15 @@ namespace grb {
// of the use of the above internal convenience function

/** \internal No implementation details; simply delegates */
template< Descriptor descr = descriptors::no_operation,
typename DataType1, typename DataType2 >
template<
Descriptor descr = descriptors::no_operation,
typename DataType1, typename DataType2,
typename RIT1, typename CIT1, typename NIT1,
typename RIT2, typename CIT2, typename NIT2
>
RC set(
Matrix< DataType1, BSP1D > &out,
const Matrix< DataType2, BSP1D > &in,
Matrix< DataType1, BSP1D, RIT1, CIT1, NIT1 > &out,
const Matrix< DataType2, BSP1D, RIT2, CIT2, NIT2 > &in,
const Phase &phase = EXECUTE
) noexcept {
assert( phase != TRY );
Expand All @@ -88,11 +95,13 @@ namespace grb {
/** \internal Simply delegates to process-local backend. */
template<
Descriptor descr = descriptors::no_operation,
typename DataType1, typename DataType2, typename DataType3
typename DataType1, typename DataType2, typename DataType3,
typename RIT1, typename CIT1, typename NIT1,
typename RIT2, typename CIT2, typename NIT2
>
RC set(
Matrix< DataType1, BSP1D > &out,
const Matrix< DataType2, BSP1D > &mask,
Matrix< DataType1, BSP1D, RIT1, CIT1, NIT1 > &out,
const Matrix< DataType2, BSP1D, RIT2, CIT2, NIT2 > &mask,
const DataType3 &val,
const Phase &phase = EXECUTE
) noexcept {
Expand All @@ -111,15 +120,20 @@ namespace grb {
/** \internal Simply delegates to process-local backend */
template<
Descriptor descr = descriptors::no_operation,
class MulMonoid,
typename OutputType, typename InputType1, typename InputType2,
class MulMonoid
typename RIT1, typename CIT1, typename NIT1,
typename RIT2, typename CIT2, typename NIT2,
typename RIT3, typename CIT3, typename NIT3
>
RC eWiseApply( Matrix< OutputType, BSP1D > &C,
const Matrix< InputType1, BSP1D > &A,
const Matrix< InputType2, BSP1D > &B,
RC eWiseApply(
Matrix< OutputType, BSP1D, RIT1, CIT1, NIT1 > &C,
const Matrix< InputType1, BSP1D, RIT2, CIT2, NIT2 > &A,
const Matrix< InputType2, BSP1D, RIT3, CIT3, NIT3 > &B,
const MulMonoid &mul,
const Phase phase = EXECUTE,
const typename std::enable_if< !grb::is_object< OutputType >::value &&
const typename std::enable_if<
!grb::is_object< OutputType >::value &&
!grb::is_object< InputType1 >::value &&
!grb::is_object< InputType2 >::value &&
grb::is_monoid< MulMonoid >::value,
Expand Down Expand Up @@ -154,17 +168,22 @@ namespace grb {
/** \internal Simply delegates to process-local backend */
template<
Descriptor descr = descriptors::no_operation,
class Operator,
typename OutputType, typename InputType1, typename InputType2,
class Operator
typename RIT1, typename CIT1, typename NIT1,
typename RIT2, typename CIT2, typename NIT2,
typename RIT3, typename CIT3, typename NIT3
>
RC eWiseApply( Matrix< OutputType, BSP1D > &C,
const Matrix< InputType1, BSP1D > &A,
const Matrix< InputType2, BSP1D > &B,
RC eWiseApply(
Matrix< OutputType, BSP1D, RIT1, CIT1, NIT1 > &C,
const Matrix< InputType1, BSP1D, RIT2, CIT2, NIT2 > &A,
const Matrix< InputType2, BSP1D, RIT3, CIT3, NIT3 > &B,
const Operator &op,
const Phase phase = EXECUTE,
const typename std::enable_if< !grb::is_object< OutputType >::value &&
!grb::is_object< InputType1 >::value && !
grb::is_object< InputType2 >::value &&
const typename std::enable_if<
!grb::is_object< OutputType >::value &&
!grb::is_object< InputType1 >::value &&
!grb::is_object< InputType2 >::value &&
grb::is_operator< Operator >::value,
void >::type * const = nullptr
) {
Expand Down
Loading

0 comments on commit 3b5e72f

Please sign in to comment.