Adjust + reduce on atomics for Chapel POI change #453
Conversation
Arkouda runs `+ reduce` on arrays of atomic ints. For that, it redefines `+` and `+=` on atomics in user code to read() from each atomic int. Starting with chapel-lang/chapel#16158 this does not work any longer. Instead, implement this reduction using a forall loop with a reduce intent, reading the atomics within the loop. This works with both Chapel 1.22 release and Chapel master. Testing: `make test-python test-chapel` with 1.22 homebrew and with Chapel master under gasnet. Minor performance improvement left as future work: specialize the pattern: ```chpl var hist = makeDistArray(bins,int); hist = lHist; return hist; ``` fot the case when `MyDmap` == `Dmap.defaultRectangular`, by returning `lHist` (or `gHist`) directly.
|
Hi @mhmerrill — Arkouda has been breaking in testing against our master branch this week due to changes to how point of instantiation is handled for generic functions to prevent surprises and hijacking. This change will fix it, so would be nice to merge if you're open to it. The reason POI comes into play is that the code implementing the reduction doesn't necessarily have access to the scope where the Arkouda-overloaded + operations are defined. This is a question we're continuing to wrestle with ("How should visibility of operators be handled?") and relates to a question I asked you a month or so ago about whether you had preferences between expressing operator overloads as standalone functions or methods. Bottom line: I don't think the design questions here are fully resolved yet, but it would be helpful to us to merge this PR to keep our testing working and on-track if you're OK with it. |
|
In discussing this, Mike asked "Should the language just have support for |
|
Mike, thank you for reviewing my two PRs. |
Arkouda runs
+ reduceon arrays of atomic ints. For that, it redefines+and+=on atomics in user code to read() from each atomic int. Starting with chapel-lang/chapel#16158 this does not work any longer.Instead, implement this reduction using a forall loop with a reduce intent, reading the atomics within the loop. This works with both Chapel 1.22 release and Chapel master.
Testing:
make test-python test-chapelwith 1.22 homebrew and with Chapel master under gasnet.Minor performance improvement left as future work: specialize the pattern:
for the case when
MyDmap==Dmap.defaultRectangular, by returninglHist(orgHist) directly.