Symbolic ConservedVars and _cls_has_array_context_attr=True

[majosm]

Some of the tests construct ConservedVars instances containing pymbolic expressions, and thus don't have array contexts. This is causing warnings to be emitted, e.g.:

mirgecom/fluid.py:235: DeprecationWarning: No array context was found. This will be an error starting in July of 2022. If you would like the function to return None if no array context was found, use get_container_context_recursively_opt.
    return get_container_context_recursively(self.mass)

This comes from the ConservedVars.array_context attribute. I don't think we can use get_container_context_recursively_opt here without changing how we set _cls_has_array_context_attr (which IIRC is not something we want to do?)

Not sure exactly how to avoid this. Do we need something like a pymbolic array context?

cc @inducer

[inducer]

IMO, this is further evidence that inducer/arraycontext#162 should happen.

Where is get_container_context_recursively_opt called, and how come it's not a big deal that it returns None?

[MTCam]

IMO, this is further evidence that inducer/arraycontext#162 should happen.

Where is get_container_context_recursively_opt called, and how come it's not a big deal that it returns None?

We don't call get_container_context_recursively_opt, the warning is telling us that we should if the container sometimes won't have an array context. We call get_container_context_recursively here:

mirgecom/mirgecom/fluid.py

Line 232 in c9d43c8

@property

and we set this:

mirgecom/mirgecom/fluid.py

Line 54 in c9d43c8

_cls_has_array_context_attr=True,

The main concern that I had is that there must be some performance implication of setting things up to use get_container_context_recursively_op, otherwise why not just always return None when the container doesn't have one? In real runs, we always have a context afaik, and I didn't want to just switch it up to use get_container_context_recursively_opt until we had spoken with you about it.

Do you have any guidance on that?

[inducer]

The performance concern is just for the traversal of the container structure, which doesn't happen at execution time when we're using lazy eval, so I think that's a minor concern.

Do we need something like a pymbolic array context?

I'm not sure that makes sense.

I'm still thinking that inducer/arraycontext#162 should happen soon-ish. That means the overload logic that @MTCam is asking about will have to be a bit more generous in terms of what it accepts, but I think that's an OK price to pay for the removal of something that ostensibly makes no sense.