-
Notifications
You must be signed in to change notification settings - Fork 488
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
Filter tensor arguments from traced model. #5689
Filter tensor arguments from traced model. #5689
Conversation
This PR filters tensor arguments from the list of arguments that would be given to the model. **Problem:** dynamo bridge assumed all arguments were tensors. **Solution:** filter tensor arguments so that we correctly collect tensor information.
Thanks, can you add a test cast that would fail without this change? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR! LGTM pending the CI tests.
test/dynamo/test_bridge.py
Outdated
module = Emb() | ||
module.to(device) | ||
|
||
@torch.compile(backend="openxla") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, openxla
doesn't have this problem. This only happens with openxla_eval
. Since we are trying to get rid of openxla_eval
, are we still interested in merging this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is aot-autograd handles the argument. What's the non-tensor input passed to openxla_eval
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, exactly. One of the inputs passed was a custom class that inherited from nn.Embedding
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me check with Brian whether it is possible for us to get non-tensor input after aot-autograd
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually @bdhirsh any thought?
I think it is safer to have this change in, I am going to merge it. |
* Filter tensor arguments from traced model. This PR filters tensor arguments from the list of arguments that would be given to the model. **Problem:** dynamo bridge assumed all arguments were tensors. **Solution:** filter tensor arguments so that we correctly collect tensor information. * Add test. * Fix lint issues. * Simplified test. * Use `openxla` instead of `openxla_eval` backend. * Rename variables for readability. * Use `openxla_eval` instead of `openxla`.
* Filter tensor arguments from traced model. This PR filters tensor arguments from the list of arguments that would be given to the model. **Problem:** dynamo bridge assumed all arguments were tensors. **Solution:** filter tensor arguments so that we correctly collect tensor information. * Add test. * Fix lint issues. * Simplified test. * Use `openxla` instead of `openxla_eval` backend. * Rename variables for readability. * Use `openxla_eval` instead of `openxla`.
* Filter tensor arguments from traced model. This PR filters tensor arguments from the list of arguments that would be given to the model. **Problem:** dynamo bridge assumed all arguments were tensors. **Solution:** filter tensor arguments so that we correctly collect tensor information. * Add test. * Fix lint issues. * Simplified test. * Use `openxla` instead of `openxla_eval` backend. * Rename variables for readability. * Use `openxla_eval` instead of `openxla`.
* Filter tensor arguments from traced model. This PR filters tensor arguments from the list of arguments that would be given to the model. **Problem:** dynamo bridge assumed all arguments were tensors. **Solution:** filter tensor arguments so that we correctly collect tensor information. * Add test. * Fix lint issues. * Simplified test. * Use `openxla` instead of `openxla_eval` backend. * Rename variables for readability. * Use `openxla_eval` instead of `openxla`.
* Filter tensor arguments from traced model. This PR filters tensor arguments from the list of arguments that would be given to the model. **Problem:** dynamo bridge assumed all arguments were tensors. **Solution:** filter tensor arguments so that we correctly collect tensor information. * Add test. * Fix lint issues. * Simplified test. * Use `openxla` instead of `openxla_eval` backend. * Rename variables for readability. * Use `openxla_eval` instead of `openxla`.
* Filter tensor arguments from traced model. This PR filters tensor arguments from the list of arguments that would be given to the model. **Problem:** dynamo bridge assumed all arguments were tensors. **Solution:** filter tensor arguments so that we correctly collect tensor information. * Add test. * Fix lint issues. * Simplified test. * Use `openxla` instead of `openxla_eval` backend. * Rename variables for readability. * Use `openxla_eval` instead of `openxla`.
This PR filters tensor arguments, when collecting tensor information, from the list of arguments that would be given to the model.
Problem: dynamo bridge assumed all arguments were tensors.
Solution: filter tensor arguments so that we correctly collect tensor information.