Skip to content

Conversation

@driazati
Copy link
Contributor

@driazati driazati commented Nov 20, 2019

This PR adds (un)pickling support for c10::Device. It also adds torch.device as a type annotation for device attributes.

Differential Revision: D18664421

This PR adds (un)pickling support for `c10::Device`. It also adds `torch.device` as a type annotation for device attributes.
@driazati driazati requested a review from apaszke as a code owner November 20, 2019 01:10
@facebook-github-bot facebook-github-bot added the oncall: jit Add this issue/PR to JIT oncall triage queue label Nov 20, 2019
@resistor
Copy link
Contributor

Could we reduce forward-compat problems by splitting this into a reader change that we ship first, and a writer change that we ship second?

@resistor resistor self-requested a review November 20, 2019 17:25
@driazati
Copy link
Contributor Author

This doesn't change any existing behavior (someone has to notice this feature and then go and try to serialize a Device), so it won't break anything that already exists, so I think it's pretty low risk as is

@driazati driazati requested a review from zdevito November 22, 2019 21:39
Copy link
Contributor

@zdevito zdevito left a comment

Choose a reason for hiding this comment

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

Looks good, comments inline.

stack_.emplace_back(c10::Device(device_string.toStringRef()));
});
stack_.emplace_back(int64_t(globals_.size() - 1));
return opcode;
Copy link
Contributor

Choose a reason for hiding this comment

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

The early returns that have accumulated in this branch of the switch statement should be cleaned up. It is not clear to me why some cases break, others early return, and some push the globals reference number into the stack. Ideally, nothing inside of the switch statement should be returning early. Perhaps put the global logic in its own function for clarity.

wuhuikx pushed a commit to wuhuikx/pytorch that referenced this pull request Jan 30, 2020
Summary:
This PR adds (un)pickling support for `c10::Device`. It also adds `torch.device` as a type annotation for device attributes.
](https://our.intern.facebook.com/intern/diff/18664421/)
Pull Request resolved: pytorch#30131

Pulled By: driazati

Differential Revision: D18664421

fbshipit-source-id: 64378fb42b2d1bbe2bd86259e5ed10f24b5d1e49
@facebook-github-bot facebook-github-bot deleted the driazati/device branch July 13, 2020 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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