Skip to content

Commit fc4e54a

Browse files
authored
Resolve internal issue #557: bugs in eWiseApply (#105)
The `grb::eWiseApply` had several issues: 1. all variants did not check properly for `ILLEGAL` arguments in the presence of the dense descriptor; 2. in one variant (`vector<-scalar<-vector, operator`) performed an in-place update while the primitive should be out-of-place; 3. the implementation variant that is driven by the nonzero pattern of the mask, in the non-vectorised coda of its implementation, did not check whether both input vectors had an element assigned; 4. in the vector-driven sparse variant and for monoids, conditions could arise where nonzeroes in the output vector may be skipped, while their values would be correctly recorded. These issues triggered rarely (as in no known "natural" occurrence), and are all fixed in this MR. This MR also - includes some minor code style fixes throughout the files touched; - updates the dispatch-to-inplace detection methods used within `eWiseApply` to use `getID` instead of pointer-based logic; - removes some optimisations for cases that would never occur (now also asserted as such); - handles trivial operations on empty vectors faster. Thanks to Aristeidis for detecting issues 2-4!
1 parent 4485bdb commit fc4e54a

3 files changed

Lines changed: 206 additions & 100 deletions

File tree

include/graphblas/bsp1d/blas1.hpp

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1477,7 +1477,9 @@ namespace grb {
14771477

14781478
// check if can delegate to dense variant
14791479
const size_t n = size( z );
1480-
if( (descr & descriptors::dense) || nnz( x ) == n ) {
1480+
if( (descr & descriptors::dense) || (
1481+
nnz( x ) == n && nnz( z ) == n
1482+
) ) {
14811483
return eWiseApply< descr | descriptors::dense >(
14821484
z, x, beta, monoid.getOperator(), phase
14831485
);
@@ -1488,7 +1490,7 @@ namespace grb {
14881490
return MISMATCH;
14891491
}
14901492
if( descr & descriptors::dense ) {
1491-
if( nnz( x ) < n ) {
1493+
if( nnz( x ) < n || nnz( z ) < n ) {
14921494
return ILLEGAL;
14931495
}
14941496
}
@@ -1566,7 +1568,9 @@ namespace grb {
15661568

15671569
// check if can delegate to dense variant
15681570
const size_t n = size( z );
1569-
if( (descr & descriptors::dense) || nnz( y ) == n ) {
1571+
if( (descr & descriptors::dense) || (
1572+
nnz( y ) == n && nnz( z ) == n
1573+
) ) {
15701574
return eWiseApply< descr | descriptors::dense >(
15711575
z, alpha, y, monoid.getOperator(), phase
15721576
);
@@ -1577,7 +1581,7 @@ namespace grb {
15771581
return MISMATCH;
15781582
}
15791583
if( descr & descriptors::dense ) {
1580-
if( nnz( y ) < n ) {
1584+
if( nnz( y ) < n || nnz( z ) < n ) {
15811585
return ILLEGAL;
15821586
}
15831587
}
@@ -1657,7 +1661,9 @@ namespace grb {
16571661

16581662
// check if we can delegate to dense variant
16591663
const size_t n = size( z );
1660-
if( (descr & descriptors::dense) || (nnz( x ) == n && nnz( y ) == n) ) {
1664+
if( (descr & descriptors::dense) || (
1665+
nnz( x ) == n && nnz( y ) == n && nnz( z ) == n
1666+
) ) {
16611667
return eWiseApply< descr | descriptors::dense >(
16621668
z, x, y, monoid.getOperator(), phase
16631669
);
@@ -1773,7 +1779,7 @@ namespace grb {
17731779
return MISMATCH;
17741780
}
17751781
if( descr & descriptors::dense ) {
1776-
if( nnz( y ) < n || nnz( mask ) < n ) {
1782+
if( nnz( y ) < n || nnz( mask ) < n || nnz( z ) < n ) {
17771783
return ILLEGAL;
17781784
}
17791785
}
@@ -1879,6 +1885,9 @@ namespace grb {
18791885
if( nnz( x ) < n ) {
18801886
return ILLEGAL;
18811887
}
1888+
if( nnz ( z ) < n ) {
1889+
return ILLEGAL;
1890+
}
18821891
}
18831892

18841893
// handle trivial resize phase
@@ -1979,7 +1988,7 @@ namespace grb {
19791988
return MISMATCH;
19801989
}
19811990
if( descr & descriptors::dense ) {
1982-
if( nnz( x ) < n || nnz( y ) < n ) {
1991+
if( nnz( x ) < n || nnz( y ) < n || nnz( z ) < n ) {
19831992
return ILLEGAL;
19841993
}
19851994
if( nnz( mask ) < n ) {
@@ -2499,7 +2508,7 @@ namespace grb {
24992508
return MISMATCH;
25002509
}
25012510
if( descr & descriptors::dense ) {
2502-
if( nnz( x ) < n || nnz( y ) < n ) {
2511+
if( nnz( x ) < n || nnz( y ) < n || nnz( z ) < n ) {
25032512
return ILLEGAL;
25042513
}
25052514
}
@@ -2563,7 +2572,7 @@ namespace grb {
25632572
return MISMATCH;
25642573
}
25652574
if( descr & descriptors::dense ) {
2566-
if( nnz( y ) < n ) {
2575+
if( nnz( y ) < n || nnz( z ) < n ) {
25672576
return ILLEGAL;
25682577
}
25692578
}
@@ -2624,7 +2633,7 @@ namespace grb {
26242633
return MISMATCH;
26252634
}
26262635
if( descr & descriptors::dense ) {
2627-
if( nnz( x ) < n ) {
2636+
if( nnz( x ) < n || nnz( z ) < n ) {
26282637
return ILLEGAL;
26292638
}
26302639
}

0 commit comments

Comments
 (0)