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

[Core] zero-copy serializer for pytorch #12344

Merged
merged 6 commits into from
Nov 27, 2020

Conversation

suquark
Copy link
Member

@suquark suquark commented Nov 24, 2020

Why are these changes needed?

When shifted from PyArrow, we lost zero-copy serialization for torch tensors. This PR not only recovers the zero-copy serialization, it also fixes several bugs in the original serializer.

Related issue number

Closes #4855
Closes #12317

Checks

  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

@suquark
Copy link
Member Author

suquark commented Nov 24, 2020

This PR should be ok. I'll add more tests later.

@suquark suquark changed the title [WIP] zero-copy serializer for pytorch Zero-copy serializer for pytorch Nov 25, 2020
@suquark suquark changed the title Zero-copy serializer for pytorch [Core] zero-copy serializer for pytorch Nov 25, 2020
@suquark suquark added the tests-ok The tagger certifies test failures are unrelated and assumes personal liability. label Nov 26, 2020
@suquark suquark assigned simon-mo and unassigned pcmoritz Nov 26, 2020
@suquark suquark merged commit 0a505ca into ray-project:master Nov 27, 2020
@suquark suquark deleted the torch_zerocopy_serialization branch November 27, 2020 00:10
krfricke pushed a commit to krfricke/ray that referenced this pull request Nov 28, 2020
richardliaw pushed a commit that referenced this pull request Nov 28, 2020
suquark added a commit to suquark/ray that referenced this pull request Nov 29, 2020
* zero-copy serializer for pytorch

* address possible bottleneck

* add tests & device support

(cherry picked from commit 0a505ca)
@sven1977
Copy link
Contributor

The torch import in serialization_addons.py breaks our RLlib test (test_dependency_torch), which checks whether everything would still work, even if torch were not installed (e.g. for tf-only users).
Solution: Use from ray.rllib.utils.framework import try_import_torch; torch, nn = try_import_torch().

suquark added a commit that referenced this pull request Nov 30, 2020
* [Core] zero-copy serializer for pytorch (#12344)

* zero-copy serializer for pytorch

* address possible bottleneck

* add tests & device support

(cherry picked from commit 0a505ca)

* add environmental variables

* update doc
amogkam added a commit to amogkam/ray that referenced this pull request Dec 1, 2020
rkooo567 pushed a commit that referenced this pull request Dec 1, 2020
@simon-mo
Copy link
Contributor

@suquark any chance we can bring in back in?

@msaroufim
Copy link

+1 this was a really cool feature

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests-ok The tagger certifies test failures are unrelated and assumes personal liability.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Core] Zero Copy Read not working for PyTorch Tensor Pytorch's requires_grad attribute isn't serialized
5 participants