10000 [dynamo][pytree][2/N] make CXX pytree traceable: `tree_flatten` / `tree_unflatten` / `tree_structure` by XuehaiPan · Pull Request #137398 · pytorch/pytorch · GitHub
[go: up one dir, main page]

Skip to content

[dynamo][pytree][2/N] make CXX pytree traceable: tree_flatten / tree_unflatten / tree_structure #137398

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 64 commits into from

Conversation

[ghstack-poisoned]
Copy link
pytorch-bot bot commented Oct 5, 2024

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/137398

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit 17d9823 with merge base 7435f57 (image):
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

[ghstack-poisoned]
XuehaiPan added a commit that referenced this pull request Oct 5, 2024
…ee_unflatten`

ghstack-source-id: 1b725e9
Pull Request resolved: #137398
[ghstack-poisoned]
XuehaiPan added a commit that referenced this pull request Oct 5, 2024
…ee_unflatten`

ghstack-source-id: 7e69b37
Pull Request resolved: #137398
[ghstack-poisoned]
XuehaiPan added a commit to XuehaiPan/pytorch that referenced this pull request Oct 5, 2024
…ee_unflatten` / `tree_structure`

ghstack-source-id: 7deed05
Pull Request resolved: pytorch#137398
[ghstack-poisoned]
XuehaiPan added a commit to XuehaiPan/pytorch that referenced this pull request Oct 5, 2024
…ee_unflatten` / `tree_structure`

ghstack-source-id: ad8c86d
Pull Request resolved: pytorch#137398
[ghstack-poisoned]
XuehaiPan added a commit to XuehaiPan/pytorch that referenced this pull request Oct 5, 2024
…ee_unflatten` / `tree_structure`

ghstack-source-id: a47eb32
Pull Request resolved: pytorch#137398
[ghstack-poisoned]
XuehaiPan added a commit to XuehaiPan/pytorch that referenced this pull request Oct 5, 2024
…ee_unflatten` / `tree_structure`

ghstack-source-id: d441e86
Pull Request resolved: pytorch#137398
[ghstack-poisoned]
XuehaiPan added a commit to XuehaiPan/pytorch that referenced this pull request Oct 5, 2024
…ee_unflatten` / `tree_structure`

ghstack-source-id: 1025e14
Pull Request resolved: pytorch#137398
@XuehaiPan XuehaiPan requested a review from angelayi December 9, 2024 12:22
@XuehaiPan
Copy link
Collaborator Author

The thing that I want is someone to spend a week or two doing a careful enumeration of all differences between C++ pytree and Python pytree (I don't have a high-level view of this right now)

The existing C++ pytree and Python pytree share the same API and function signature. There are tests in our CI to ensure this. More, the pytree tests and dynamo tests are both tested against the Python pytree, C++ pytree, and the new public pytree API (a thin wrapper around the underlying pytree implementation).

@XuehaiPan
< 10000 /svg> Copy link
Collaborator Author

Hi @jansel, could you take a look at this PR and the next PR in the stack? Then the follow-ups can be reviewed and merged separately.

pytorchmergebot pushed a commit that referenced this pull request Dec 12, 2024
bluenote10 pushed a commit to bluenote10/pytorch that referenced this pull request Dec 14, 2024
bluenote10 pushed a commit to bluenote10/pytorch that referenced this pull request Dec 14, 2024
XuehaiPan added a commit to XuehaiPan/pytorch that referenced this pull request Dec 18, 2024
XuehaiPan added a commit to XuehaiPan/pytorch that referenced this pull request Dec 18, 2024
XuehaiPan added a commit to XuehaiPan/pytorch that referenced this pull request Dec 19, 2024
XuehaiPan added a commit to XuehaiPan/pytorch that referenced this pull request Dec 19, 2024
XuehaiPan added a commit to XuehaiPan/pytorch that referenced this pull request Dec 24, 2024
XuehaiPan added a commit to XuehaiPan/pytorch that referenced this pull request Dec 24, 2024
XuehaiPan added a commit to XuehaiPan/pytorch that referenced this pull request Dec 25, 2024
XuehaiPan added a commit to XuehaiPan/pytorch that referenced this pull request Dec 25, 2024
XuehaiPan added a commit to XuehaiPan/pytorch that referenced this pull request Dec 30, 2024
XuehaiPan added a commit to XuehaiPan/pytorch that referenced this pull request Dec 30, 2024
XuehaiPan added a commit to XuehaiPan/pytorch that referenced this pull request Jan 7, 2025
XuehaiPan added a commit to XuehaiPan/pytorch that referenced this pull request Jan 7, 2025
XuehaiPan added a commit to XuehaiPan/pytorch that referenced this pull request Jan 10, 2025
XuehaiPan added a commit to XuehaiPan/pytorch that referenced this pull request Jan 10, 2025
@github-actions github-actions bot deleted the gh/XuehaiPan/177/head branch January 12, 2025 02:10
XuehaiPan added a commit to XuehaiPan/pytorch that referenced this pull request Jan 15, 2025
XuehaiPan added a commit to XuehaiPan/pytorch that referenced this pull request Jan 15, 2025
Comment on lines +115 to +116
F438
@dataclass(frozen=True)
class PyTreeSpec:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@XuehaiPan, if I'm understanding this correctly, you're adding a polyfill for cxx_pytree.tree_flatten such that it will return an instance of this PyTreeSpec class. I'm not sure this works if we are trying to return a PyTreeSpec from a torch.compile'd function: does it create an instance of this "class PyTreeSpec" object, or does it create an instance of torch.utils._cxx_pytree.TreeSpec?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In a compiled function, it returns torch._dynamo.polyfills.pytree.PyTreeSpec. This class provides the exactly same interfaces with torch.utils._cxx_pytree.TreeSpec.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah this is wrong, see the following:

import torch
import torch.utils._cxx_pytree as pytree

@torch.compile(backend="eager", fullgraph=True)
def f(x, y):
    vals, spec = pytree.tree_flatten(x)
    return vals, spec, y.sin()

y = torch.randn(3)
x = [1, [2, [3, 4]]]
vals, spec, _ = f(x, y)
this_doesnt_work = pytree.tree_unflatten(vals, spec)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. It may cause problems if we only compile part of the program.

There is an in-progress polyfill infra for C++ classes.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants
0