[pbrubeck]

Fixes #5524.

Summary

Kernel calls (pyop3/insn/visitors.py, expand_transforms for CalledFunction):

 MAX_RW:     pack  t = x               unpack  x = max(x, t)
 MIN_RW:     pack  t = x               unpack  x = min(x, t)
+MAX_WRITE:  pack  t = dtype min       unpack  x = max(x, t)
+MIN_WRITE:  pack  t = dtype max       unpack  x = min(x, t)

The temporary starts from the identity of the reduction, so the entries that the kernel does not write leave x unchanged. exec.py uses the same identities on the leaves.

Assignments (pyop3/insn/base.py, AbstractAssignment):

 argument_intents
-  WRITE -> WRITE, else assert INC -> INC
+  assignment_type_as_intent(assignment_type)   # MAX -> MAX_WRITE, MIN -> MIN_WRITE
 __str__
+  MAX -> "max=", MIN -> "min="

Evidence

• Before:

  assemble(interpolate(x + y, V, access=op3.MIN_WRITE))
    NotImplementedError: Intent Intent.MIN_WRITE not handled
  op3.loop(c := mesh.iter("cell"), f.dat[mesh.closure(c)].assign(1.0, "max"), eager=True)
    AssertionError  (argument_intents)
  

  After: both reproducers from pyop3: MIN_WRITE/MAX_WRITE kernel calls and max/min assignments in loops fail #5524 give the correct values on 1 and 3 ranks.

• New tests, all fail without the fix (except the existing MAX case) and pass with it:

  • tests/pyop3/insn/test_loop.py::test_reduction_assignment[max/min]
  • tests/firedrake/regression/test_interpolate.py::test_interpolate_periodic_coords_reduction[MAX/MAX_WRITE/MIN_WRITE], which replaces test_interpolate_periodic_coords_max and runs on nprocs=1 and nprocs=3. With MIN_WRITE the nodes on the periodic seam take the value 0, with MAX/MAX_WRITE they take 1.

Merge Danger

Door: two-way

Blast Radius: small

Only the MIN_WRITE/MAX_WRITE intents and the "min"/"max" assignments change; these paths raised an error before.

---

AI assistance: this PR was written with Claude Code (Claude Opus 5.5).

🤖 Generated with Claude Code

[connorjward]

Seems weird to use (m - 1) / 2. It is confusing to read because it makes me think that there's something special about that value.

[pbrubeck]

I changed to an explicit 4.5

[connorjward]

MIN and MAX_RW and MIN_RW?

Definitely should check MIN but not sure about the others. I had to introduce RW and WRITE variants in pyop3 but the distinction isn't important at the Firedrake level.

[pbrubeck]

MIN, MAX are just the same as MIN_RW, MAX_RW

[connorjward]

No. This is exactly the difference between MAX_RW and MAX_WRITE.

[pbrubeck]

Could you elaborate please?

[connorjward]

The entire difference between these operations is what we pack on the way into the local kernel. Is it the actual data or is it something null.

t <- -inf
kernel(t)
dat <- max(dat, t)

vs

t <- dat
kernel(t)
dat <- max(dat, t)

[pbrubeck]

The standard Interpolate(expr, V, access=WRITE) generates a kernel whose output tensor should be zeroed at the start. For each degree of freedom we compute a weighted sum of the expr at the element's quadrature points.

tensor[i] = 0
for j in range(dual_mat.shape[1]):
    tensor[i] += dual_mat[i, j] * expr(pts[j])

For Interpolate(expr, V, access=MAX) one would like to set -inf on the dat first, which means that we are no longer able to accumulate on the output. Instead we need to call the WRITE kernel on a temporary, on which we still acumulate the weighted sum.

tensor[i] = -inf

def kernel(temp)
    temp[i] = 0
    for j in range(dual_mat.shape[1]):
        temp[i] += dual_mat[i, j] * expr(pts[j])
    
 tensor[i] = max(tensor[i], temp[i])

[connorjward]

I have just played around with this. If you delete requires_zeroed_output_arguments then the interpolate tests still pass

[pbrubeck]

The flag is here for #5258, it is not needed for the bespoke Interpolate codegen, only for the unified TSFC approach.

[pbrubeck]

I think #5258 should use generate_loopy(..., return_increments=(acces == INC))

[pbrubeck]

Done 276218a