Conversation
|
MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅ |
| ) | ||
| self._nodes[node.name] = node | ||
| self._validated = False | ||
| if isinstance(node, CacheableComposerNode): |
There was a problem hiding this comment.
It looks like two things inherently go together: the node being a CacheableComposerNode and the cache_path being a path. The former without the latter is a degenerate case that should probably fail early, and the latter without the former is pointless.
Is that something worth capturing in one place, instead of having two tests of the cache object's integrity? (i.e. isinstance(node, CacheableComposerNode) and then if self._cache_path )
(unless I'm misunderstanding NullCache and it is a legitimate caching node that behaves differently from a non-caching node).
There was a problem hiding this comment.
I don't think a CacheableComposerNode without a cache path is a degenerate case! If there is no cache path, then the cacheable node behaves as a regular node (the NullCache is no-op, nothing is ever written).
|
See my comment in the other PR when you get a chance before merging this one. |
3 participants
Every node has its own cache but the caching mechanics are managed by the composer.