New Issue: Can accumulate be called after combine for reductions?

Notifications Issues
1

16459, “ronawho”, “Can accumulate be called after combine for reductions?”, “2020-09-24T18:56:42Z”

I’m looking at bug in arkouda’s mink/maxk that was made more likely with https://github.com/chapel-lang/chapel/pull/16401, but I can repro with 1.22 as well.

The maxk code uses a heap to keep track of the max k values while accumulating, and then as an optimization when combining it un-heapifys by sorting the buffer and merges sorted buffers. However, I’m seeing some additional calls to accumulate after that sorting, and the data structure is no longer suitable for the heaped accumulate so it’s getting wrong answers. I wasn’t sure if accumulate after combine was legal or not so wanted to ask here.

Here’s a reproducer (just took the technote reduction and added halts if accumulate is called after combine)

class PlusReduceOp: ReduceScanOp {
  type eltType;
  var value: eltType;
  var startedCombine = false;

  proc identity {
    return 0: eltType;
  }

  proc accumulate(elm) {
    if startedCombine then halt("accum after combine");
    value = value + elm;
  }

  proc accumulateOntoState(ref state, elm) {
    if startedCombine then halt("accum after combine");
    state = state + elm;
  }

  proc initialAccumulate(outerVar) {
    value = value + outerVar: eltType;
  }

  proc combine(other: borrowed PlusReduceOp) {
    startedCombine = true;
    value = value + other.value;
  }

  proc generate() return value;

  proc clone() return new unmanaged PlusReduceOp(eltType=eltType);
}

var A = [1000, 200, 30, 4];
var sum: int;
forall elm in A with (PlusReduceOp reduce sum) {
  sum += elm;  // equivalently:  sum reduce= elm;
}
writeln(sum);