search

inducer / arraycontext

Choose your favorite numpy-workalike!
6 stars 11 forks source link

Is passing frozen arrays to call_loopy okay? #42

Closed inducer closed 3 years ago

inducer commented 3 years ago

This is common practice throughout much of meshmode and grudge at the moment. The Pytato array context has to work around it:

https://github.com/inducer/arraycontext/blob/152511624bc0d5ab8ca1e64ac160a73d030544d5/arraycontext/impl/pytato/__init__.py#L92-L94

On the one hand, it's at least a little bit contradictory that this would be OK for call_loopy, but not OK for things like frozen * thawed. On the other hand, the array context is explicit when call_loopy is used. I'm a bit torn. Thoughts?

cc @matthiasdiener @kaushikcfd @alexfikl

kaushikcfd commented 3 years ago

Currently for both the array context implementations there is a negligible cost associated with thawing a frozen array, and it's quite clear from the context that the user simply "forgot" to type out actx.thaw, so, I'm fine with the automatic promotion. But, if in the future there is a chance that thawing a frozen array isn't cheap then we should amend things early, I can't imagine such of an arraycontext implementation.

not OK for things like frozen * thawed

TBH, In the PyOpenCLArrayContext the notion that binary ops are only done between thawed arrays is also not true, but luckily there aren't many abusers of such behavior in the downstream packages.

In [9]: a = actx.zeros(10, dtype=float)

In [10]: b = actx.zeros(10, dtype=float) + 1

In [11]: b1 = actx.freeze(b)

In [12]: a + b1
Out[12]: cl.Array([1., 1., 1., 1., 1., 1., 1., 1., 1., 1.])

Related: https://github.com/inducer/meshmode/issues/107

inducer commented 3 years ago

Thanks for reminding me! I agree that this was resolved in https://github.com/inducer/meshmode/issues/107 in the affirmative, and I don't see a good reason to reverse that decision.