Repository navigation
[BugFix] Implement pytree flatten_with_keys for lazy stacks - #2640
Open
YeonwooSung wants to merge 2 commits into
Open
YeonwooSung wants to merge 2 commits into
YeonwooSung wants to merge 2 commits into
Conversation
tree_flatten_with_path on a lazy stack raises NotImplementedError. The test checks that its spec matches tree_flatten, that member keys are SequenceKey in order, and that unflatten rebuilds the stack.
Return the same values and context as _lazy_tensordict_flatten, with each member wrapped in SequenceKey, so tree_flatten_with_path matches tree_flatten.
This branch has not been deployed
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.
Problem
_lazy_td_flatten_with_keyswas registered as the pytreeflatten_with_keys_fnforLazyStackedTensorDict, but it only raisedNotImplementedError.tree_flatten_with_path(andtree_map_with_path) therefore failed on a lazy stack.torch.export compares the spec of
tree_flatten_with_pathon the call inputs with the spec oftree_flattenon the example inputs. Dense tensordicts already keep those specs aligned by returning the same context from_td_flatten_with_keysas from_tensordict_flatten. A lazy stack did not.Fix
_lazy_td_flatten_with_keysnow calls_lazy_tensordict_flattenand returns that same context, with each member wrapped inSequenceKey(i)in member order. Unflatten is unchanged.Tests
TestPyTree.test_pytree_lazy_flatten_with_keyschecks that:tree_flatten_with_pathdoes not raise on a lazy stacktree_flattenSequenceKeyin member order, including a nested member and an exclusive keytree_unflattenrebuilds a lazy stack with the samestack_dim, values, names, and batch sizeBefore the fix the new test failed with
NotImplementedError. After the fix:TestPyTree(15 tests) also passed. Ruff check and format are clean on the touched files.