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

Remove .impl_UNBOXED() and functionalities associated with it #49220

Closed
wants to merge 48 commits into from

Conversation

smessmer
Copy link
Contributor

@smessmer smessmer commented Dec 11, 2020

Stack from ghstack:

Since all ops are c10-full, we can remove .impl_UNBOXED now.
This also removes the ability of KernelFunction or CppFunction to store unboxedOnly kernels.

Differential Revision: D25490225

NOTE FOR REVIEWERS: This PR has internal Facebook specific changes or comments, please review them on Phabricator!

Since all ops are c10-full, we can remove .impl_UNBOXED now.
This also removes the ability of KernelFunction or CppFunction to store unboxedOnly kernels.

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

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D25490225/)!

[ghstack-poisoned]
… it"

Since all ops are c10-full, we can remove .impl_UNBOXED now.
This also removes the ability of KernelFunction or CppFunction to store unboxedOnly kernels.

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

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D25490225/)!

[ghstack-poisoned]
smessmer added a commit that referenced this pull request Dec 11, 2020
Pull Request resolved: #49220

Since all ops are c10-full, we can remove .impl_UNBOXED now.
This also removes the ability of KernelFunction or CppFunction to store unboxedOnly kernels.
ghstack-source-id: 118382014

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

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D25490225/)!
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Dec 11, 2020

💊 CI failures summary and remediations

As of commit 9c99d93 (more details on the Dr. CI page):


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

🕵️ 2 new failures recognized by patterns

The following CI failures do not appear to be due to upstream breakages:

See CircleCI build pytorch_linux_bionic_py3_8_gcc9_coverage_test1 (1/2)

Step: "Run tests" (full log | diagnosis details | 🔁 rerun)

Jan 06 19:03:08 [E request_callback_no_python.cpp:636] Received error while processing request type 258: RuntimeError: Can not pickle torch.futures.Future
Jan 06 19:03:08 At:
Jan 06 19:03:08   /opt/conda/lib/python3.8/site-packages/torch/distributed/rpc/internal.py(120): serialize
Jan 06 19:03:08   /opt/conda/lib/python3.8/site-packages/torch/distributed/rpc/internal.py(172): serialize
Jan 06 19:03:08 
Jan 06 19:03:08 [E request_callback_no_python.cpp:636] Received error while processing request type 258: RuntimeError: Can not pickle torch.futures.Future
Jan 06 19:03:08 
Jan 06 19:03:08 At:
Jan 06 19:03:08   /opt/conda/lib/python3.8/site-packages/torch/distributed/rpc/internal.py(120): serialize
Jan 06 19:03:08   /opt/conda/lib/python3.8/site-packages/torch/distributed/rpc/internal.py(172): serialize
Jan 06 19:03:08 
Jan 06 19:03:08 [E request_callback_no_python.cpp:636] Received error while processing request type 258: RuntimeError: Can not pickle torch.futures.Future
Jan 06 19:03:08 
Jan 06 19:03:08 At:
Jan 06 19:03:08   /opt/conda/lib/python3.8/site-packages/torch/distributed/rpc/internal.py(120): serialize
Jan 06 19:03:08   /opt/conda/lib/python3.8/site-packages/torch/distributed/rpc/internal.py(172): serialize
Jan 06 19:03:08 
Jan 06 19:03:08 [W tensorpipe_agent.cpp:547] RPC agent for worker0 encountered error when reading incoming request from worker3: EOF: end of file (this is expected to happen during shutdown)
Jan 06 19:03:08 [W tensorpipe_agent.cpp:547] RPC agent for worker0 encountered error when reading incoming request from worker1: EOF: end of file (this is expected to happen during shutdown)
Jan 06 19:03:08 [W tensorpipe_agent.cpp:547] RPC agent for worker2 encountered error when reading incoming request from worker0: EOF: end of file (this is expected to happen during shutdown)
Jan 06 19:03:08 [W tensorpipe_agent.cpp:547] RPC agent for worker0 encountered error when reading incoming request from worker2: EOF: end of file (this is expected to happen during shutdown)
Jan 06 19:03:08 ok (2.559s)

See CircleCI build pytorch_windows_vs2019_py36_cuda10.1_test1 (2/2)

Step: "Test" (full log | diagnosis details | 🔁 rerun)

RuntimeError: test_nn failed!
  test_ReflectionPad2d (__main__.TestNN) ... ok (1.171s)
  test_ReflectionPad2d_alert_nondeterministic_cuda (__main__.TestNN) ... ok (0.018s)
  test_ReflectionPad2d_cuda (__main__.TestNN) ... ok (0.013s)
  test_ReplicationPad1d (__main__.TestNN) ... ok (0.051s)
  test_ReplicationPad1d_alert_nondeterministic_cuda (__main__.TestNN) ... ok (0.018s)
  test_ReplicationPad1d_cuda (__main__.TestNN) ... Traceback (most recent call last):
  File "run_test.py", line 910, in <module>
    main()
  File "run_test.py", line 889, in main
    raise RuntimeError(err_message)
RuntimeError: test_nn failed!

(base) circleci@PACKER-5FD865C5 C:\Users\circleci\project\test>if ERRORLEVEL 1 exit /b 1 
+ cleanup
+ retcode=1
+ set +x


Exited with code exit status 1


1 job timed out:

  • pytorch_linux_bionic_py3_8_gcc9_coverage_test1

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 to the (internal) Dr. CI Users group.

This comment has been revised 501 times.

… it"

Since all ops are c10-full, we can remove .impl_UNBOXED now.
This also removes the ability of KernelFunction or CppFunction to store unboxedOnly kernels.

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

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D25490225/)!

[ghstack-poisoned]
… it"

Since all ops are c10-full, we can remove .impl_UNBOXED now.
This also removes the ability of KernelFunction or CppFunction to store unboxedOnly kernels.

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

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D25490225/)!

[ghstack-poisoned]
… it"

Since all ops are c10-full, we can remove .impl_UNBOXED now.
This also removes the ability of KernelFunction or CppFunction to store unboxedOnly kernels.

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

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D25490225/)!

[ghstack-poisoned]
… it"

Since all ops are c10-full, we can remove .impl_UNBOXED now.
This also removes the ability of KernelFunction or CppFunction to store unboxedOnly kernels.

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

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D25490225/)!

[ghstack-poisoned]
… it"

Since all ops are c10-full, we can remove .impl_UNBOXED now.
This also removes the ability of KernelFunction or CppFunction to store unboxedOnly kernels.

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

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D25490225/)!

[ghstack-poisoned]
… it"

Since all ops are c10-full, we can remove .impl_UNBOXED now.
This also removes the ability of KernelFunction or CppFunction to store unboxedOnly kernels.

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

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D25490225/)!

[ghstack-poisoned]
… it"

Since all ops are c10-full, we can remove .impl_UNBOXED now.
This also removes the ability of KernelFunction or CppFunction to store unboxedOnly kernels.

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

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D25490225/)!

[ghstack-poisoned]
… it"

Since all ops are c10-full, we can remove .impl_UNBOXED now.
This also removes the ability of KernelFunction or CppFunction to store unboxedOnly kernels.

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

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D25490225/)!

[ghstack-poisoned]
… it"

Since all ops are c10-full, we can remove .impl_UNBOXED now.
This also removes the ability of KernelFunction or CppFunction to store unboxedOnly kernels.

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

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D25490225/)!

[ghstack-poisoned]
… it"

Since all ops are c10-full, we can remove .impl_UNBOXED now.
This also removes the ability of KernelFunction or CppFunction to store unboxedOnly kernels.

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

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D25490225/)!

[ghstack-poisoned]
… it"

Since all ops are c10-full, we can remove .impl_UNBOXED now.
This also removes the ability of KernelFunction or CppFunction to store unboxedOnly kernels.

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

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D25490225/)!

[ghstack-poisoned]
… it"

Since all ops are c10-full, we can remove .impl_UNBOXED now.
This also removes the ability of KernelFunction or CppFunction to store unboxedOnly kernels.

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

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D25490225/)!

[ghstack-poisoned]
… it"

Since all ops are c10-full, we can remove .impl_UNBOXED now.
This also removes the ability of KernelFunction or CppFunction to store unboxedOnly kernels.

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

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D25490225/)!

[ghstack-poisoned]
… it"

Since all ops are c10-full, we can remove .impl_UNBOXED now.
This also removes the ability of KernelFunction or CppFunction to store unboxedOnly kernels.

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

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D25490225/)!

[ghstack-poisoned]
… it"

Since all ops are c10-full, we can remove .impl_UNBOXED now.
This also removes the ability of KernelFunction or CppFunction to store unboxedOnly kernels.

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

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D25490225/)!

[ghstack-poisoned]
… it"

Since all ops are c10-full, we can remove .impl_UNBOXED now.
This also removes the ability of KernelFunction or CppFunction to store unboxedOnly kernels.

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

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D25490225/)!

[ghstack-poisoned]
… it"

Since all ops are c10-full, we can remove .impl_UNBOXED now.
This also removes the ability of KernelFunction or CppFunction to store unboxedOnly kernels.

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

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D25490225/)!

[ghstack-poisoned]
… it"

Since all ops are c10-full, we can remove .impl_UNBOXED now.
This also removes the ability of KernelFunction or CppFunction to store unboxedOnly kernels.

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

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D25490225/)!

[ghstack-poisoned]
… it"

Since all ops are c10-full, we can remove .impl_UNBOXED now.
This also removes the ability of KernelFunction or CppFunction to store unboxedOnly kernels.

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

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D25490225/)!

[ghstack-poisoned]
… it"

Since all ops are c10-full, we can remove .impl_UNBOXED now.
This also removes the ability of KernelFunction or CppFunction to store unboxedOnly kernels.

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

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D25490225/)!

[ghstack-poisoned]
Copy link
Contributor

@ezyang ezyang left a comment

Choose a reason for hiding this comment

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

Good riddance!!!

@ezyang
Copy link
Contributor

ezyang commented Jan 4, 2021

Guessing this will need an XLA companion?

@smessmer
Copy link
Contributor Author

smessmer commented Jan 4, 2021

Guessing this will need an XLA companion?

yes, pytorch/xla#2714 needs to be landed before this

… it"

Since all ops are c10-full, we can remove .impl_UNBOXED now.
This also removes the ability of KernelFunction or CppFunction to store unboxedOnly kernels.

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

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D25490225/)!

[ghstack-poisoned]
… it"

Since all ops are c10-full, we can remove .impl_UNBOXED now.
This also removes the ability of KernelFunction or CppFunction to store unboxedOnly kernels.

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

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D25490225/)!

[ghstack-poisoned]
… it"

Since all ops are c10-full, we can remove .impl_UNBOXED now.
This also removes the ability of KernelFunction or CppFunction to store unboxedOnly kernels.

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

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D25490225/)!

[ghstack-poisoned]
… it"

Since all ops are c10-full, we can remove .impl_UNBOXED now.
This also removes the ability of KernelFunction or CppFunction to store unboxedOnly kernels.

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

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D25490225/)!

[ghstack-poisoned]
… it"

Since all ops are c10-full, we can remove .impl_UNBOXED now.
This also removes the ability of KernelFunction or CppFunction to store unboxedOnly kernels.

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

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D25490225/)!

[ghstack-poisoned]
@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 4a14020.

@facebook-github-bot facebook-github-bot deleted the gh/smessmer/286/head branch January 10, 2021 15:17
hwangdeyu pushed a commit to hwangdeyu/pytorch that referenced this pull request Jan 14, 2021
…h#49220)

Summary:
Pull Request resolved: pytorch#49220

Since all ops are c10-full, we can remove .impl_UNBOXED now.
This also removes the ability of KernelFunction or CppFunction to store unboxedOnly kernels.
ghstack-source-id: 119450489

Test Plan: waitforsandcastle

Reviewed By: ezyang

Differential Revision: D25490225

fbshipit-source-id: 32de9d591e6a842fe18abc82541580647e9cfdad
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants