adding reduce to Ops.jl

[tharittk]

I deliberately did not test on non-commutative operators—it will fail when tested against Julia's reduce. I read the discussion on the internet that it is supposed to be so, i.e., reduce should take a commutative operator; otherwise, the result will be non-deterministic, especially in a parallel system.

[giordano]

I deliberately did not test on non-commutative operators—it will fail when tested against Julia's reduce.

Does that mean that the reduce method introduced here has different semantics than Julia's reduce? That'd be unideal, asi it can lead to hard-to-track bugs when people write generic code and Reactant suddenly changes their meaning (similar for example to #755)

[mofeing]

one thing to note is that methods in Ops should replicate the semantics of StableHLO (and possibly other MLIR dialects), not Julia.

the adaptation from StableHLO to Julia semantics is done in the method specializations outside the Ops module.

[giordano]

one thing to note is that methods in Ops should replicate the semantics of StableHLO (and possibly other MLIR dialects), not Julia.

Ah, yes, that's a good point. This is not extending Base.reduce, so that's fine (although the same name is slightly confusing 😅).

[mofeing]

... (although the same name is slightly confusing 😅).

yep 😅, but it's because it wraps the stablehlo.reduce op. as a rule of thumb, you should not import Ops but call them explicitly like Ops.reduce to avoid confusion.

[avik-pal]

Can you follow up with a PR to update the mapreduce impl to call this function

[tharittk]

Sure I'll do that

[avik-pal]

the cuda reduce tests are failing

[tharittk]

Based on my initial investigation, it seems to be about how the init_values is handled. In case of CPU - via Reactant.set_default_backend("cpu"), there is no issue.

But when using GPU, if the init_values is set to be 1 for * and 0 for +, for example, it will be OK. Other than that, it looks like the GPU applies init_values multiple times.

For example, if we start with
A = [1 3; 2 4;;; 5 7; 6 8;;; 9 11; 10 12]

with init_values = 2, and dim = [3] then
we have [(15+2) (21+2); (18+2) (24+2)] = [17 23; 20 26] -- both CPU and GPU version agree

with init_values = 2, and dim = [1, 3] then
We should have [(33 + 2) (45+2)] -- init_value only applied once (this is what Julia's reduce() gives also)
but with stablehlo + GPU, we get [(17+20) (23+26)] -- effectively init_values is applied twice

I am not sure if this is a sematic issue or something else.

What I think happens is stablehlo broadcasts the init_values at the beginning and do reduce while Julia's reduce does the reduce and then broadcasts the init_values to whatever size is after reduce at the end