Trouble with container handling in op reductions

[MTCam]

I'm having a bit of trouble with container reductions when attempting to switch from EagerDGDisc to op namespace.

This pattern is consistently used in grudge for container reductions:

grudge/grudge/reductions.py

Line 255 in 6c9e7b2

if not isinstance(vec, DOFArray):

One problem is that this pattern is never what one wants when the array container components are each independent physical quantities. For example, when using a CV object in mirgecom, one would never want this: max(cv.mass, cv.energy, cv.momentum[0], cv.momentum[1], .... ), however that is what these grudge reductions are wont to do.

@inducer says: (Snipped part about empty reductions, see #266.)

Discussing with @majosm , he suggested maybe we should add a flag to indicate whether we want to reduce across components of an array container. We don't want that in the vast majority of cases. Such a flag would help with the first problem. Maybe switching to a map_array_container approach would address the second part and then perform a 2nd reduction across components when the flag is set.

Just wanting to start a discussion with this. Any ideas @inducer?

[inducer]

• I made the empty reductions question a separate issue, see Do not fail for empty reductions #266.
• I've converted this to a discussion since there's nothing actionable here just yet.

I think a flag to prevent summing over containers is a bit of a blunt instrument. For example, DOFArrays are also containers. We are currently only dealing with discretizations that only have a single group, but that may change. I feel that implicitly reducing over groups in DOFArrays is almost always the desired outcome. Similarly, summing over object arrays is likely always desired. Even summing over things like

https://github.com/inducer/grudge/blob/6c9e7b251b9d723cc0fc98f0682c214d55c0a22e/examples/wave/wave-op-mpi.py#L62-L65=

is meaningful, because $\int|u|^2+|v|^2$ is a type of energy that's conserved by the conservation law (IIRC). That said, I agree that summing over ConservedVars in mirgecom is unlikely to be helpful to anyone.

Maybe what we want is a way for containers to declare whether reductions across them are sensible?

[MTCam]

Maybe what we want is a way for containers to declare whether reductions across them are sensible?

This would be terrific, and seems like it would be easy to do. Is it?

[inducer]

It's not particularly hard, but the choice of mechanism deserves some care:

• Stick an attribute into the container class. (Probably not awesome since we don't want to, e.g., monkeypatch np.ndarray with our custom attribute garbage.)
• Have a set of container classes allowing reduction somehwere, maybe in arraycontext
• Pass a set of container classes to each reduction, potentially with convencience wrappers for specific choices

I'm sure there's more.

[inducer]

@majosm I'd like your feedback here too once you're back! :)

[MTCam]

Ok, so some nuance there; I was afraid of that. We have a natural work-around that is substantially less trouble. We can re-model our calls for reductions of containers like this:
https://github.com/illinois-ceesd/mirgecom/blob/117714c97485aa2f8220d11694a1584a43e39e44/mirgecom/simutil.py#L316

It's just a little bit awkward, but gives some time to figure out the best course of action (if any) for grudge.

[inducer]

I was afraid of that

Nothing to be afraid of, we just need to choose something!