Skip to content
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

improve tree mismatch error in tree_map #23771

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mattjj
Copy link
Collaborator

@mattjj mattjj commented Sep 19, 2024

import jax
import jax.numpy as jnp

jax.tree.map(lambda x: x, {'a': {'b': [1,2,3]}}, {'a': {'b': [1,2,3,4]}})

Before:

ValueError: List arity mismatch: 4 != 3; list: [1, 2, 3, 4].

After:

ValueError: in tree_map, at the key path ['a']['b'] the first tree has a list of length 3
but the second tree has a list of length 4, so the lengths do not match:
[1, 2, 3]
vs
[1, 2, 3, 4]

jax.tree.map(lambda x: x, [1,2,3] * 50, [1,2,3,4] * 50)

After:

ValueError: in tree_map, at the tree root the first tree has a list of length 150
but the second tree has a list of length 200, so the lengths do not match.

Intended improvements:

  1. show key path
  2. show both mismatching subtree values if they're short (rather than just the latter tree's)

TODO (maybe in follow-ups...):

  • validate on dicts, custom pytree nodes, etc
  • tests

@sshleifer
Copy link

Thank you so much way better!

@IvyZX
Copy link
Collaborator

IvyZX commented Oct 30, 2024

This change looks fantastic! Is there anything blocking it to be landed?

@gnecula
Copy link
Collaborator

gnecula commented Oct 31, 2024

I am also happy to see more thinking here. I have found equality_errors very useful, but hard to use. I recommend searching for usage of tree_util.equality_errors and seeing how we can simplify usage.

@mattjj
Copy link
Collaborator Author

mattjj commented Nov 2, 2024

Thanks, @IvyZX ! TBH I don't remember if there were blockers or if I just failed to follow up. Let me see if I can land it now...

@gnecula I agree. This PR is just a small step, and as you say we could really use some deeper thinking about error utilities.

@mattjj mattjj requested a review from IvyZX November 2, 2024 20:51
@mattjj mattjj marked this pull request as ready for review November 2, 2024 20:51
Copy link
Collaborator

@IvyZX IvyZX left a comment

Choose a reason for hiding this comment

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

Thank you so much!

@google-ml-butler google-ml-butler bot added kokoro:force-run pull ready Ready for copybara import and testing labels Nov 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pull ready Ready for copybara import and testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants