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

[torch.bind_symbolic_shape] Fix verifier for shapeSymbol detection #3751

Merged

Conversation

meshtag
Copy link
Member

@meshtag meshtag commented Oct 1, 2024

The op can be valid with no attached shape symbols if they are not required by the corresponding affine map. Fix the verifier to consider number of arguments for both.

The op can be valid with no attached shape symbols if they are not required
by the corresponding affine map. Fix the verifier to consider number of
arguments for both.
Signed-off-by: Prathamesh Tagore <prathamesh+1@polymagelabs.com>
Copy link
Collaborator

@stellaraccident stellaraccident left a comment

Choose a reason for hiding this comment

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

Good catch. Thanks. Do you happen to have a minimal example that produces this behavior. It's not obvious to me and we may want to add a test.

@meshtag
Copy link
Member Author

meshtag commented Oct 2, 2024

Thanks for the review.

Do you happen to have a minimal example that produces this behavior. It's not obvious to me and we may want to add a test.

I got this error in graph 2 (as partitioned by dynamo) of nano_llava model (model spec here). At the moment, I do not have a python spec which would directly lead to this (it is not obvious to me as well, tbh).

PS: I would require your help to land this (if we want to merge in this state). Thanks.

@stellaraccident
Copy link
Collaborator

Thanks for the review.

Do you happen to have a minimal example that produces this behavior. It's not obvious to me and we may want to add a test.

I got this error in graph 2 (as partitioned by dynamo) of nano_llava model (model spec here). At the moment, I do not have a python spec which would directly lead to this (it is not obvious to me as well, tbh).

PS: I would require your help to land this (if we want to merge in this state). Thanks.

Thanks. It's a reasonable thing to fix -- I just try to keep a mental tally on where things come up.

@stellaraccident stellaraccident merged commit 617c1c7 into llvm:main Oct 2, 2024
3 checks passed
@meshtag meshtag deleted the prathamesh/fix_torch_bind_symbolic_shape branch October 3, 2024 03:09
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