Skip to content

Tentative solution to pytree#27

Merged
samuela merged 1 commit into
samuela:mainfrom
mavenlin:pytree
Sep 2, 2025
Merged

Tentative solution to pytree#27
samuela merged 1 commit into
samuela:mainfrom
mavenlin:pytree

Conversation

@mavenlin

Copy link
Copy Markdown
Contributor

We may move the discussions on pytree here, there are some future PRs that would depend on a systematic solution of this.

Comment thread torch2jax/__init__.py Outdated
@samuela

samuela commented Aug 22, 2025

Copy link
Copy Markdown
Owner

ah this is a cute idea. i like it if it works! so iiuc the idea is that we replace

# use the torch's tree_map, because out is generated from torch code
return torch.utils._pytree.tree_map(lambda x: x.value if isinstance(x, Torchish) else x, out)
with _tree_coerce and then it automatically handles pytorch pytree types as we encounter them? i wonder if this will work inside of jax.jit?

@mavenlin
mavenlin force-pushed the pytree branch 3 times, most recently from b39a65a to aa7f23f Compare August 23, 2025 14:06
@mavenlin

Copy link
Copy Markdown
Contributor Author

This PR is the prerequisite for functions like topk, sort, max to be properly handled as they have custom return types under torch.return_types., which needs to be systematically handled with the tree node registration.

@samuela samuela left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

this contains a lot of formatting changes that have no impact on functionality. let's remove those before merging

Comment thread torch2jax/__init__.py Outdated
Comment thread torch2jax/__init__.py Outdated
Comment thread torch2jax/__init__.py Outdated
Comment thread torch2jax/__init__.py Outdated
Comment thread tests/test_core.py
@samuela
samuela merged commit 4e6d65f into samuela:main Sep 2, 2025
1 check passed
@samuela

samuela commented Sep 2, 2025

Copy link
Copy Markdown
Owner

this is a cleaner solution than i thought would be possible, thanks @mavenlin !

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants