Deprecate with_container_arithmetic's bcast_numpy_array arg

[kaushikcfd]

Passing both 'bcast_numpy_array' and '_bcast_actx_array_types' was
ill-defined. For example, in the case of an ArrayContext whose thawed
array type is np.ndarray the specification would contradict between
broadcasting the argument numpy_array to return an object array *OR*
peforming the operation with every leaf array.
Consider the example below,
(
- 'Foo: ArrayContainer' whose arithmetic routines are
generated by `with_container_arithmetic(bcast_numpy=True,
_bcast_actx_array_types=True)`
- 'actx: ArrayContextT' for whom `np.ndarray` is a valid thawed array
type.
)
Foo(DOFArray(actx, [38*actx.ones(3, np.float64)])) + np.array([3, 4, 5])
could be either of:
- array([Foo(DOFArray([array([41, 41, 41])])),
Foo(DOFArray([array([42, 42, 42])])),
Foo(DOFArray([array([43, 43, 43])]))]), OR,
- Foo(DOFArray(actx, array([41, 42, 43])))

Draft because

• Numpy arrays to be broadcasted as obj arrays pyrometheus/pyrometheus#35
• Do not broadcast scalar numpy arrays illinois-ceesd/mirgecom#514

[inducer]

cc @majosm (I think this would be worth a look once you're back)

[majosm]

Not sure if I like the change in illinois-ceesd/mirgecom#514... requiring the application programmer to distinguish between numpy arrays of dtype float and object arrays containing floats seems less than ideal.

Would it be enough to just forbid setting both bcast_numpy_array and _bcast_actx_array_type to True? Maybe _bcast_actx_array_type shouldn't be True by default?

[kaushikcfd]

@majosm: Yep, the interface won't be as brief as before, but I can see folks preferring Numpy over PyOpenCL in MIRGE-Com (for development purposes), as PyOpenCL arrays aren't quite fully implemented when it comes to features like broadcasting, non-contiguous copies, etc. (@thomasgibson took this route while prototyping some of his stuff).

And, if we intend to support a Numpy-based array context, casting them into object arrays before using them for arithmetic is a deal-breaker for its implementation.

[inducer]

I think an array context that just wraps numpy is well worth having. (I can already think of quite a few use cases myself, and @thomasgibson's case demonstrates that we'd want to allow that even in mirgecom.)

As far as I can tell, it's not straightforward from looking at a numpy array in bcast_numpy_array processing whether the array is intended to be a container (and thus broadcast) or not. Forcing those arrays to be object arrays is sensible IMO, even if it makes the user code less slick-looking. Again IMO, we erred too much on the side of magic here and scored a bit of an own goal in terms of backward compatibility.

If there's an elegant way that gets us numpy array contexts and avoids the compat break, I'd be curious to hear about it. But as long as it an either-or between plain-numpy array contexts and slicker-looking broadcasting, I'll take the former, I think.

[majosm]

As far as I can tell, it's not straightforward from looking at a numpy array in bcast_numpy_array processing whether the array is intended to be a container (and thus broadcast) or not. Forcing those arrays to be object arrays is sensible IMO, even if it makes the user code less slick-looking. Again IMO, we erred too much on the side of magic here and scored a bit of an own goal in terms of backward compatibility.

If there's an elegant way that gets us numpy array contexts and avoids the compat break, I'd be curious to hear about it. But as long as it an either-or between plain-numpy array contexts and slicker-looking broadcasting, I'll take the former, I think.

Is there a use case for _bcast_actx_array_type=True for things like DOFArray and ConservedVars? I just tried disabling it, and it didn't seem to break anything.

[kaushikcfd]

Is there a use case for _bcast_actx_array_type=True for things like DOFArray and ConservedVars? I just tried disabling it, and it didn't seem to break anything.

Yep, if we use PytatoArrayContext for the project https://github.com/illinois-ceesd/drivers_y1-nozzle, we do see failures if not _bcast_actx_array_type. As the compiled operator makes "time" (a scalar in PyOpenCLArrayContext) as a placeholder array, and, there are locations in the nozzle-driver where the "time" variable must be broadcasted to all the arrays in a container.

[kaushikcfd]

About _bcast_actx_array_type: there was also some discussion in #49.

[majosm]

Is there a use case for _bcast_actx_array_type=True for things like DOFArray and ConservedVars? I just tried disabling it, and it didn't seem to break anything.

Yep, if we use PytatoArrayContext for the project https://github.com/illinois-ceesd/drivers_y1-nozzle, we do see failures if not _bcast_actx_array_type. As the compiled operator makes "time" (a scalar in PyOpenCLArrayContext) as a placeholder array, and, there are locations in the nozzle-driver where the "time" variable must be broadcasted to all the arrays in a container.

Ah, ok. Yeah, I can't really think of any way around this then.