Repository navigation
Make it so array containers do not know their array context? #162
Description
This is aimed particularly at DOFArray.array_context, which is AFAIK the only instance of this, but a pervasive one. This change, if implemented, is rather break-the-world-y, so we would have to do this slowly and carefully.
Why is it worth doing? It's forcing arraycontext.container.traversal.{freeze,thaw} to be @singledispatch over the container type, just for a way to remove the stored .array_context. For batch-freezing in the array context, this is inconvenient (see discussion in #158).
What does it break?
grudgegets its array context from the arrays it's passed for many operations.- Any function that's
actx.compiled, because that's its sole source of array cotnext.
That seems like a lot of breakage for limited gain...
@alexfikl @kaushikcfd @majosm Thoughts?
Activity
majosm commentedon May 9, 2022
Would the individual arrays still know their array context? Or would the array context need to be passed into every function as a separate argument?
inducer commentedon May 9, 2022
They already do not.
pyopencl.array.Arrayhas no idea what an array context is. At some point, a numpy array will also be a valid array. It, too, has never heard of an array context. (IOW, we can't be that intrusive.)majosm commentedon May 9, 2022
Oh right, I forgot. Yeah, this change sounds like it might be rough. Can't say I understand the
@singledispatchissue well enough to say whether it's worthwhile or not.I don't suppose it would be feasible to make a wrapper layer for thawed arrays? i.e.
thawreturns something that contains an array and behaves like one but also holds anarray_contextreference?inducer commentedon May 9, 2022
Given the amount of
isinstance(..., something_like_an_array)flying around, I feel like that would be even more destructive than what's in the OP...majosm commentedon May 10, 2022
Do we do that a lot? Most of the
isinstances I can think of check for containers likeDOFArray, not the underlying array types themselves. I was picturing wrapping the underlying arrays, and then usingget_array_context_recursivelyto retrieve an array context for containers likeDOFArrays, etc. Again, I definitely don't know the full scope of array context usage well enough to say if something like this makes any sense, I'm just throwing out ideas. Feel free to ignore. 🙂inducer commentedon May 10, 2022
I appreciate your pariticipating in the discussion! Mostly, I'm not too optimistic about these wrapper-y approaches based on past experience, because there always seem to be hard-to-fix corner cases where the wrapper only imperfectly pretends to be the underlying array. I agree that that's not a very tangible criticism of the approach. :)
inducer commentedon Jun 10, 2022
Upon further thought, I think this one is a non-issue, as users can pass in an appropriate array context via
partialor a closure.inducer commentedon Jul 16, 2024
FWIW, I'm more or less forcing a decision on this at https://github.com/inducer/grudge/pull/353/files#r1679781502.