Skip to content

Commit 208f2df

Browse files
anyzelmananhansson
authored andcommitted
Clarify the dense descriptor (#38)
The documentation of the dense descriptor was incorrect and could do with clarifying corner cases. This MR adds such documentation, and also moves an exposed internal include from the header file to the corresponding `descriptors.cpp`. It also clarifies the `toString' output of the dense descriptor, and applies minor code style fixes. This MR corresponds to, and addresses, internal issue #485. Thanks to Aristeidis for reporting.
1 parent 868616c commit 208f2df

2 files changed

Lines changed: 49 additions & 15 deletions

File tree

include/graphblas/descriptors.hpp

Lines changed: 42 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -24,21 +24,31 @@
2424
* @date 15 March, 2016
2525
*/
2626

27-
#include <sstream>
28-
#include <string>
29-
3027
#ifndef _H_GRB_DESCRIPTOR
3128
#define _H_GRB_DESCRIPTOR
3229

30+
#include <string>
31+
32+
3333
namespace grb {
3434

3535
/**
3636
* Descriptors indicate pre- or post-processing for some or all of the
37-
* arguments to a GraphBLAS call.
37+
* arguments to an ALP/GraphBLAS call. An example is to transpose the input
38+
* matrix during a sparse matrix--vector multiplication:
39+
* <tt>grb::mxv< grb::descriptors::transpose_matrix >( y, A, x, ring );</tt>
40+
* the above thus computes \f$ y \to y + A^Tx \f$ and not \f$ y \to y + Ax \f$.
41+
*
42+
* Such pre-processing often happens on-the-fly, without significant overhead
43+
* to the primitive costings in any of its cost dimensions -- work, intra- and
44+
* inter-process data movement, synchronisations, and memory usage.
3845
*
39-
* They can be combined using bit-wise operators. For instance, to both
40-
* indicate the matrix needs be transposed and the mask needs be
41-
* inverted, the following descriptor can be passed:
46+
* \note If the application of a descriptor is \em not without significant
47+
* overhead, a backend \em must clearly indicate so.
48+
*
49+
* Descriptors may be combined using bit-wise operators. For instance, to both
50+
* indicate the matrix needs be transposed and the mask needs be inverted, the
51+
* following descriptor can be passed:
4252
* <tt> transpose_matrix | invert_mask </tt>
4353
*/
4454
typedef unsigned int Descriptor;
@@ -107,9 +117,31 @@ namespace grb {
107117
static constexpr Descriptor structural_complement = structural | invert_mask;
108118

109119
/**
110-
* Indicates all vectors used in a computation are dense. This is a hint that
111-
* might affect performance but will never affect the semantics of the
112-
* computation.
120+
* Indicates that all input vectors to an ALP/GraphBLAS primitive are
121+
* structurally dense.
122+
*
123+
* If a user passes this descriptor but one or more vectors input to the call
124+
* are \em not structurally dense, then #ILLEGAL shall be returned.
125+
*
126+
* \warning <em>All vectors</em> includes any vectors that operate as masks.
127+
* Thus if the primitive is to operate with structurally sparse masks
128+
* but with otherwise dense vectors, then the dense descriptor may
129+
* \em not be defined.
130+
*
131+
* \warning For in-place operations with vector outputs --which are all
132+
* ALP/GraphBLAS primitives with vector outputs except grb::set and
133+
* grb::eWiseApply-- the output vector is also an input vector. Thus
134+
* passing this descriptor to such primitive indicates that also the
135+
* output vector is structurally dense.
136+
*
137+
* \warning Vectors with explicit zeroes (under the semiring passed to the
138+
* related primitive) will be computed with explicitly.
139+
*
140+
* The benefits of using this descriptor whenever possible are two-fold:
141+
* 1) less run-time overhead as code handling sparsity is disabled;
142+
* 2) smaller binary sizes as code handling structurally sparse vectors is
143+
* not emitted (unless required elsewhere).
144+
* The consistent use of this descriptor is hence strongly encouraged.
113145
*/
114146
static constexpr Descriptor dense = 16;
115147

src/graphblas/descriptors.cpp

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@
1717

1818
#include <graphblas/descriptors.hpp>
1919

20+
#include <sstream>
21+
22+
2023
std::string grb::descriptors::toString( const grb::Descriptor descr ) {
2124
std::ostringstream os;
2225
if( descr == 0 ) {
@@ -36,23 +39,22 @@ std::string grb::descriptors::toString( const grb::Descriptor descr ) {
3639
os << " mask must be interpreted structurally, and not by value\n";
3740
}
3841
if( descr & dense ) {
39-
os << " user guarantees all vectors in this call are dense\n";
42+
os << " user guarantees that all input vectors to this call are dense\n";
4043
}
4144
if( descr & add_identity ) {
4245
os << " an identity matrix is added to the input matrix\n";
4346
}
4447
if( descr & use_index ) {
45-
os << " instead of using input vector elements, use their index "
46-
"instead\n";
48+
os << " instead of using input vector elements, use their index instead\n";
4749
}
4850
if( descr & explicit_zero ) {
4951
os << " the operation should take zeroes into account explicitly "
5052
"when computing output\n";
5153
}
5254
if( descr & no_casting ) {
53-
os << " disallow casting between types during the requested "
54-
"computation\n";
55+
os << " disallow casting between types during the requested computation\n";
5556
}
5657
}
5758
return os.str();
5859
}
60+

0 commit comments

Comments
 (0)