Repository navigation
Repeated freezes cause repeated computation #102
Description
(Maybe this is an arraycontext issue? Not sure.)
Suppose a moderately complex expression is built and evaluated. Then it is evaluated again. Repeated codegen (most likely cached, though see inducer/pytato#163) and repeated evaluation ensues. This isn't ideal. Possibly, a symbolic array, once evaluated, should hold on to its value, to avoid reevaluation. If we don't deal with this somehow, then pytato's arrays can't be meaningfully memoized (e.g. with @memoize/@memoize_method from pytools), as, sure, they memoize the expression, but the actual evaluation is done again and again.
A simple approach to this would be simply stuff the evaluated value into a hidden attribute (_evaluated?) on the expression nodes. I'm aware that this introduces mutation into otherwise immutable objects... but it's the cache-y kind of mutability, and it doesn't change the semantics of the object in the slightest. So I suspect it's OK.
Another possible concern is the lifetime of the evaluated value (i.e. it might outstay its welcome). I don't think that'll be a big concern because
- somebody held on to the symbolic value, demonstrating that there's at least potential interest in reevaluating.
- if it becomes an issue, we could make an interface to free the stored value, enabling the memory to be freed.
Generally, codegen should stop at already-evaluated notes (and treat them as DataWrappers).
@MTCam is hitting this with cache retrievals upon freeze when exercising lazy evaluation for chemistry in https://github.com/illinois-ceesd/mirgecom.
cc @kaushikcfd
Activity
kaushikcfd commentedon Oct 13, 2021
Some other things to keep in mind:
Slightly related to arraycontext:
Personally, I've overcome this by doing a
thaw(freeze(...))the time-stepper loop in array context, as generally for me the expressions that were meant to be folded into constants were forming the sub-expressions of a bigger expression that was actually frozen. And so, the evaluation being mentioned here wouldn't be of much relevance in those cases.inducer commentedon Oct 13, 2021
Agree with all three of your points. Thinking about it more has convinced me that this actually is an arraycontext issue, and pytato probably shouldn't contribute to its solution. In particular, pytato nodes shouldn't hold references to their values (because, as you point out, they're target-specific).
A few more thoughts on this:
freeze(i.e. with compilation "on the fly", not via precompilation), a lower bound on the cost is set by needing to compare the DAG against one that was fished out of the cache. This means that computing large DAGs this way can't be efficient, so we should probably avoid it as much as possible.freezeabove which we warn about efficiency?WeakKeyDictionaryheld by the array context that keeps them alive as long as the expression is.Your third point is already covered AFAICT:
https://github.com/inducer/pytato/blob/08ba9271dcf4a61b92366800aa112e9f0d4e32dc/pytato/array.py#L1587-L1588