cache: resolve modules= against the tree instead of caching nothing - #716
Open
JadenFiotto-Kaufman wants to merge 1 commit into
Open
JadenFiotto-Kaufman wants to merge 1 commit into
JadenFiotto-Kaufman wants to merge 1 commit into
Conversation
A target string was taken as given, so a typo, a path missing the model's own name, a glob or a regex subscribed to a location nothing ever provides: an empty cache and no error. Resolve each one in Cache.__init__ and let the envoy tree raise, and say what modules= accepts for a callable or a regex, which failed with "'function' object is not iterable". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was broken
Cache.__init__builtself.targetsfrom the strings it was given withoutresolving them, then subscribed to locations nothing provides. Any string that
named no module produced an empty cache and no error:
while the correct spellings worked. A callable or an
re.Patternpassed asmodules=failed withTypeError: 'function' object is not iterable.Repro:
tasks/cache-probes-function-vectors/skills/repro_3_unknown_path_silent.py.What changed
src/nnsight/intervention/cache.py— eachmodules=entry is resolved inCache.__init__:.path, as before;AttributeErrorreading that module by attribute gives, named against thetarget it came from. A path is the whole path (the way the cache's keys are);
one missing the model's own name gets a "did you mean 'model.…'?" when adding
it back resolves. Aliases resolve too, so a renamed spelling now normalises to
the real path rather than caching nothing;
Parameter, or on.output/.input(a value nnsightserves, not a module), says so rather than being subscribed to;
TypeErrorsayingwhat
modules=accepts and that globs, regexes and predicates are not matchedagainst the tree. A lone string is treated as one target — iterating it would
otherwise take it a character at a time, which was another silent empty cache.
Docstrings for
tracer.cache(modules=...)andCache.targets, and oneparagraph in
docs/usage/cache.mdunder "Cache a subset of modules", say what apath has to be and how to select modules by pattern (filter
model.modules()yourself).
Tests
12 tests in
tests/test_interleaving.py::TestCache(the tinyEnvoy(Net())fixture that class already uses): a path-string subset and a lone path string as
the positive controls, an alias target resolving to the real path, the four
misses from the repro plus
model.l1.weight, the missing-model-name suggestion,.outputas a target, andre.compile(...)/ a lambda asmodules=.Ran with
PYTHONPATH=src:tests/test_interleaving.py— 89 passed, 10 failed; the same 10 fail onorigin/0.8untouched (they are the.save()-on-a-Tensor family, i.e. thetrace-body capture problem another item covers), 77 passed there.
tests/test_language.py -k cache— 7 passed, 1 pre-existing failure(
test_cache_per_invoke_holds_its_own_rows), identical against a pristinesrccheckout.tests/test_language.py tests/test_batching.py— 43 passed / 94 failed, bytefor byte the same counts as the untouched branch.
tests/test_fragments.py tests/test_envoy.py— 92 passed / 8 failed, same asuntouched.
No GPU or remote paths are touched by this change.
Deliberately left alone
modules=(not in a list) still iterates itschildren, because
Envoy.__iter__exists:modules=model.transformer.hmeansthe 12 blocks today, and a leaf envoy passed that way still caches nothing.
Making a lone envoy one target would change that first, documented-by-use
behaviour, so it stayed out of a validation fix.
docs/usage/cache.md's separateinclude_inputs=True/DynamicCacheproblem, which another change is addressing in the same file.
🤖 Generated with Claude Code