Support minmax reductions?

[ronawho]

We have min and max reductions, but no way to get both at once. You can get the min and max separately but you end up looking through your array twice. You can optimize this by manually writing the loop yourself, but it's not as clean/simple:

var A: [1..10] int = 1..10;

// Clean-ish, but goes through the array twice
{
  var aMin = min reduce A;
  var aMax = max reduce A;
  writeln((aMin, aMax));
}

// Uglier, but faster
{
  // I usually incorrectly write `var aMin, aMax: int`, which leads to subtle bugs
  var aMin = max(int); 
  var aMax = min(int);
  forall a in A with (min reduce aMin, max reduce aMax) {
    aMin reduce= a;
    aMax reduce= a;
  }
  writeln((aMin, aMax));
}

It seems attractive to be able to do something like:

var (aMin, aMax) = minmax reduce A;

Finding both min and max is a pretty common idiom idiom in arkouda. Reductions are fast enough that I don't think it's a major bottleneck, but it would save some time.

[bradcray]

(it also could beg the question whether we'd want a minmaxlocs reduction that was like minloc and maxloc combined).

[bradcray]

To get you started, I'd suggest looking at modules/internal/ChapelReduce.chpl, look at how the min and MinReduceOp and MaxReduceOp classes are defined, and add a similar class named minmax. Doing so ought to permit you to write minmax reduce ... test/thomasvandoren/mink.chpl (and the nearby tests that use it) may also be a good reference point. Sorry there's not better documentation on writing these operations, but it shouldn't be too hard to pattern-match against what others have done and get it working, I believe. Ask questions if not.

[bradcray]

@ronawho: PR #15062 implements minmax reductions (and should be merged soon). This made me go looking for places in Arkouda where they could be used, and I'm finding cases in:

• https://github.com/mhmerrill/arkouda/blob/master/src/ArgSortMsg.chpl
• https://github.com/mhmerrill/arkouda/blob/master/src/AryUtil.chpl
• https://github.com/mhmerrill/arkouda/blob/master/src/FindSegmentsMsg.chpl
• https://github.com/mhmerrill/arkouda/blob/master/src/HistogramMsg.chpl
• https://github.com/mhmerrill/arkouda/blob/master/src/IndexingMsg.chpl
• https://github.com/mhmerrill/arkouda/blob/master/src/RadixSortLSD.chpl
• https://github.com/mhmerrill/arkouda/blob/master/src/ReductionMsg.chpl
• https://github.com/mhmerrill/arkouda/blob/master/src/SegmentedArray.chpl

(though there may be others as well).

This raises the perennial question about whether we should open a PR to be merged after the next release, or drop in a helper file that implements the minmax reduction as a placeholder, or file an issue reminding ourselves to do this change after the next release goes out).

[ronawho]

I don't think minmax is a big bottleneck, but I'd still be in favor of getting that into arkouda. Can we just drop in a duplicate minmax class into arkouda and have that take precedence over the builtin one that will be on master, or will that be ambiguous? If it's ambiguous I'd probably just name it akminmax with a comment saying "can use builtin minmax once 1.23 is required"

[vasslitvinov]

Resolved by #15062

[bradcray]

Can we just drop in a duplicate minmax class into arkouda and have that take precedence over the builtin one that will be on master, or will that be ambiguous?

I believe if it's put into an Arkouda file that's explicitly used, this should work. But calling it akminmax may result in less confusion while achieving the same result.