Add cached interpretations and use for typeof - #716
Conversation
This reverts commit 2742dd4.
eb8680
left a comment
There was a problem hiding this comment.
Looks like a nice improvement, I just have a couple of small comments.
Also, what happened to removing internals.product_n?
| return frozenset(intp.items()) | ||
|
|
||
|
|
||
| def _term_cache(expr: Term) -> dict[object, object] | None: |
There was a problem hiding this comment.
Would it be cleaner to define this as a functools.cached_property of Term?
There was a problem hiding this comment.
This version avoids the need to update implementers of Term, but a cached property is cleaner.
There was a problem hiding this comment.
I think since we have a variety of internal and external implementers of Term, some of which don't subclass it, we should stick with the current implementation.
|
I found new uses for |
|
|
||
|
|
||
| class PureInterpretation[T, V](ObjectInterpretation[T, V]): | ||
| def __hash__(self): |
There was a problem hiding this comment.
I wonder if this should be more conservative and use object identity instead of a semantic hash of self.implementations?
There was a problem hiding this comment.
I think the semantic hash is correct and slightly more precise, but it doesn't make much difference in our use.
Adds a notion of a cached interpretation (these interpretations handle the internal operation
_is_memoized). When evaluating a term under a cached interpretation, we set a cache attribute on the term that we can reuse for later evaluations under the same interpretation.Blocked by #713 (only for the benchmark)
Before:
After:
Open design questions:
coproduct({}, i) != iin the current implementation.Previous attempt: #252