-
Notifications
You must be signed in to change notification settings - Fork 25.7k
Squeeze p2: hook up Squeeze to LazyView #73067
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
Conversation
CI Flow Status⚛️ CI FlowRuleset - Version:
|
🔗 Helpful links
💊 CI failures summary and remediationsAs of commit 7f27ad1 (more details on the Dr. CI page): 💚 💚 Looks good so far! There are no failures yet. 💚 💚 This comment was automatically generated by Dr. CI (expand for details).Please report bugs/suggestions to the (internal) Dr. CI Users group. |
torch/csrc/lazy/core/lazy_view.cpp
Outdated
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.
Unsqueeze? or Squeeze?
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.
in the backward pass of update we are undoing an operation, so for Squeeze we need to Unsqueeze and vice versa :)
Maybe I should've added Unsqueeze in the same PR. I just like small PRs !!
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.
LGTM except for the question i posted
|
@Krovatkin has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
0817fe1 to
7f27ad1
Compare
|
@Krovatkin has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Summary: This PR hooks up Squeeze op to LazyView. The end goal to reduce the instances where we need to rely on explicit shapes. In the final PR we will make `aten_ops` to use the right ViewInfo and update the lowering in ts_lowering. Pull Request resolved: #73067 Reviewed By: wconstab, mikaylagawarecki Differential Revision: D34345163 Pulled By: Krovatkin fbshipit-source-id: 6bfadedbded7521312019ead0dfc7c6a334ff0f5
Summary: This PR hooks up Squeeze op to LazyView. The end goal to reduce the instances where we need to rely on explicit shapes. In the final PR we will make `aten_ops` to use the right ViewInfo and update the lowering in ts_lowering. Pull Request resolved: pytorch/pytorch#73067 Reviewed By: wconstab, mikaylagawarecki Differential Revision: D34345163 Pulled By: Krovatkin fbshipit-source-id: 6bfadedbded7521312019ead0dfc7c6a334ff0f5 (cherry picked from commit 4b3b10fa97911c2302840b44b63619d081e0d9d4)
Summary: This PR hooks up Squeeze op to LazyView. The end goal to reduce the instances where we need to rely on explicit shapes. In the final PR we will make `aten_ops` to use the right ViewInfo and update the lowering in ts_lowering. Pull Request resolved: pytorch/pytorch#73067 Reviewed By: wconstab, mikaylagawarecki Differential Revision: D34345163 Pulled By: Krovatkin fbshipit-source-id: 6bfadedbded7521312019ead0dfc7c6a334ff0f5 (cherry picked from commit 4b3b10fa97911c2302840b44b63619d081e0d9d4)
This PR hooks up Squeeze op to LazyView.
The end goal to reduce the instances where we need to rely on explicit shapes.
In the final PR we will make
aten_opsto use the right ViewInfo and update the lowering in ts_lowering.