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

[rpc] fix RRef alias annotation #39933

Closed
wants to merge 3 commits into from

Conversation

wanchaol
Copy link
Contributor

@wanchaol wanchaol commented Jun 12, 2020

Stack from ghstack:

Fix rref related alias annotation to ensure it's not getting ebased by
the jit dce.

Differential Revision: D22015426

Fix rref related alias annotation to ensure it's not getting ebased by
the jit dce.

[ghstack-poisoned]
wanchaol added a commit that referenced this pull request Jun 12, 2020
Fix rref related alias annotation to ensure it's not getting ebased by
the jit dce.

ghstack-source-id: ef18cc7e8f87b2b15b7bde2087ba071fdb697170
Pull Request resolved: #39933
@facebook-github-bot facebook-github-bot added the oncall: jit Add this issue/PR to JIT oncall triage queue label Jun 12, 2020
@dr-ci
Copy link

dr-ci bot commented Jun 12, 2020

💊 CI failures summary and remediations

As of commit 3e7b80e (more details on the Dr. CI page):


  • 1/1 failures possibly* introduced in this PR
    • 1/1 non-CircleCI failure(s)

ci.pytorch.org: 1 failed


This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions on the GitHub issue tracker or post in the (internal) Dr. CI Users group.

See how this bot performed.

This comment has been revised 11 times.

Copy link
Contributor

@mrshenli mrshenli left a comment

Choose a reason for hiding this comment

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

Thanks for fixing promptly!

(Not suggesting changes in this PR) Do we also need to fix Future(t) here?

"aten::wait(Future(t) self) -> t",

@@ -26,7 +26,7 @@ static auto workerInfo =

RegisterOperators reg_rpc_ops(
{Operator(
"aten::to_here(RRef(t) self) -> t",
"aten::to_here(RRef(t) self) -> t(*)",
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious, do we also change t to t(*) in RRef(t)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not needed, the change to the return type is to ensure the return value is aliased to any other values so that the dce will not remove this op, the RRef(t) does not need to alias to any other value like the return value

Fix rref related alias annotation to ensure it's not getting ebased by
the jit dce.

Differential Revision: [D22015426](https://our.internmc.facebook.com/intern/diff/D22015426)

[ghstack-poisoned]
wanchaol added a commit that referenced this pull request Jun 12, 2020
Fix rref related alias annotation to ensure it's not getting ebased by
the jit dce.

ghstack-source-id: 4769468b901599e1c4fac1ded0dbf90ba3bb4f05
Pull Request resolved: #39933
@xush6528 xush6528 requested a review from rohan-varma June 12, 2020 17:16
@wanchaol
Copy link
Contributor Author

Thanks for fixing promptly!

(Not suggesting changes in this PR) Do we also need to fix Future(t) here?

good point, wait is a bit complicated with special casing and side effects, I will try to make it consistent with a follow up PR.

Fix rref related alias annotation to ensure it's not getting ebased by
the jit dce.

Differential Revision: [D22015426](https://our.internmc.facebook.com/intern/diff/D22015426)

[ghstack-poisoned]
wanchaol added a commit that referenced this pull request Jun 12, 2020
Fix rref related alias annotation to ensure it's not getting ebased by
the jit dce.

ghstack-source-id: f046efb2005d5b299cda195cca4d90bec80cc374
Pull Request resolved: #39933
@facebook-github-bot
Copy link
Contributor

@wanchaol merged this pull request in 34d1098.

RockingJavaBean added a commit to RockingJavaBean/pytorch that referenced this pull request Jun 14, 2020
Resolve the conflict of check_backward_compatibility.py
from pull request pytorch#39933

Signed-off-by: Xiong Wei <xiongw.fnst@cn.fujitsu.com>
@facebook-github-bot facebook-github-bot deleted the gh/wanchaol/112/head branch June 16, 2020 15:19
xwang233 pushed a commit to xwang233/pytorch that referenced this pull request Jun 20, 2020
Summary:
Pull Request resolved: pytorch#39933

Fix rref related alias annotation to ensure it's not getting ebased by
the jit dce.

Test Plan: Imported from OSS

Differential Revision: D22015426

Pulled By: wanchaol

fbshipit-source-id: 3e74d49fa9f88abaf662bde7be5284f01f621b98
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Merged oncall: jit Add this issue/PR to JIT oncall triage queue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants