Refactor Device to not depend on Backend. by ezyang · Pull Request #10478 · pytorch/pytorch

ezyang · 2018-08-13T18:59:08Z

- Removed Backend constructor from Device, and fixed all
  use-sites to use DeviceType::CPU instead of kCPU, or
  use a new function backendToDeviceType to perform
  the conversion.
- New method device_type() on Type; it gives you the
  underlying device type, e.g., CPU for SparseCPU.
- We add backward compatibility for kCPU/kCUDA uses,
  by introducing a new special type which is implicitly
  convertible to both DeviceType and Backend.  As long as
  you don't define a function that's overloaded on both
  DeviceType and Backend (but not on BackendOrDeviceType),
  the implicit conversions will ensure that uses
  of at::Device(at::kCPU) keep working. We fixed use-sites in
  the library, but did NOT fix sites in the test code, so that
  we can exercise this BC code.

Differential Revision: D9301861

goldsborough

Looks like a reasonable codemod! Some stylistic nits

aten/src/ATen/Backend.h

aten/src/ATen/core/typeid.cpp

aten/src/ATen/Device.h

gchanan

Do we really want/need this backwards compatibility stuff? We don't guarantee backwards compatibility with the C++ API and kCUDA, kCPU are so convenient that people might continue to use them as devices (bad!). I'd almost prefer reclaiming those names for the device types, and moving the backend short names to something else.

Thoughts @goldsborough?

aten/src/ATen/Backend.h

ezyang · 2018-08-16T14:02:23Z

Yes, I am somewhat convinced we should make kCPU and kCUDA device types. I guess I'll have to try it and see what breaks. Maybe later today.

gchanan

looks good, although tests are failing?

Summary: Pull Request resolved: pytorch#10478 - Removed Backend constructor from Device, and fixed all use-sites to use DeviceType::CPU instead of kCPU, or use a new function backendToDeviceType to perform the conversion. - New method device_type() on Type; it gives you the underlying device type, e.g., CPU for SparseCPU. - We add backward compatibility for kCPU/kCUDA uses, by introducing a new special type which is implicitly convertible to both DeviceType and Backend. As long as you don't define a function that's overloaded on both DeviceType and Backend (but not on BackendOrDeviceType), the implicit conversions will ensure that uses of at::Device(at::kCPU) keep working. We fixed use-sites in the library, but did NOT fix sites in the test code, so that we can exercise this BC code. Reviewed By: Yangqing Differential Revision: D9301861 fbshipit-source-id: 937fe15e32c2beba9235b8b26b9532ba85594635

Summary: Pull Request resolved: #10478 - Removed Backend constructor from Device, and fixed all use-sites to use DeviceType::CPU instead of kCPU, or use a new function backendToDeviceType to perform the conversion. - New method device_type() on Type; it gives you the underlying device type, e.g., CPU for SparseCPU. - We add backward compatibility for kCPU/kCUDA uses, by introducing a new special type which is implicitly convertible to both DeviceType and Backend. As long as you don't define a function that's overloaded on both DeviceType and Backend (but not on BackendOrDeviceType), the implicit conversions will ensure that uses of at::Device(at::kCPU) keep working. We fixed use-sites in the library, but did NOT fix sites in the test code, so that we can exercise this BC code. Reviewed By: Yangqing Differential Revision: D9301861 fbshipit-source-id: 9a9d88620500715c7b37e655b4fd761f6dd72716

Summary: Pull Request resolved: pytorch/pytorch#10478 - Removed Backend constructor from Device, and fixed all use-sites to use DeviceType::CPU instead of kCPU, or use a new function backendToDeviceType to perform the conversion. - New method device_type() on Type; it gives you the underlying device type, e.g., CPU for SparseCPU. - We add backward compatibility for kCPU/kCUDA uses, by introducing a new special type which is implicitly convertible to both DeviceType and Backend. As long as you don't define a function that's overloaded on both DeviceType and Backend (but not on BackendOrDeviceType), the implicit conversions will ensure that uses of at::Device(at::kCPU) keep working. We fixed use-sites in the library, but did NOT fix sites in the test code, so that we can exercise this BC code. Reviewed By: Yangqing Differential Revision: D9301861 fbshipit-source-id: 9a9d88620500715c7b37e655b4fd761f6dd72716

Summary: Pull Request resolved: pytorch#10478 - Removed Backend constructor from Device, and fixed all use-sites to use DeviceType::CPU instead of kCPU, or use a new function backendToDeviceType to perform the conversion. - New method device_type() on Type; it gives you the underlying device type, e.g., CPU for SparseCPU. - We add backward compatibility for kCPU/kCUDA uses, by introducing a new special type which is implicitly convertible to both DeviceType and Backend. As long as you don't define a function that's overloaded on both DeviceType and Backend (but not on BackendOrDeviceType), the implicit conversions will ensure that uses of at::Device(at::kCPU) keep working. We fixed use-sites in the library, but did NOT fix sites in the test code, so that we can exercise this BC code. Reviewed By: Yangqing Differential Revision: D9301861 fbshipit-source-id: 9a9d88620500715c7b37e655b4fd761f6dd72716

ezyang requested review from apaszke, colesbury, gchanan, soumith and zdevito as code owners August 13, 2018 18:59

ezyang force-pushed the export-D9301861 branch from e34c81c to 35d4ef2 Compare August 13, 2018 21:10

ezyang force-pushed the export-D9301861 branch from 35d4ef2 to ef8cf6c Compare August 13, 2018 21:52

goldsborough approved these changes Aug 13, 2018

View reviewed changes

ezyang force-pushed the export-D9301861 branch from ef8cf6c to b537699 Compare August 14, 2018 13:45

ezyang force-pushed the export-D9301861 branch from b537699 to a65e36a Compare August 14, 2018 13:52

ezyang force-pushed the export-D9301861 branch from a65e36a to bb951ca Compare August 14, 2018 19:06

ezyang force-pushed the export-D9301861 branch from bb951ca to 8a0148f Compare August 15, 2018 18:36

ezyang force-pushed the export-D9301861 branch from 8a0148f to 9d197d4 Compare August 15, 2018 21:50

gchanan reviewed Aug 15, 2018

View reviewed changes

aten/src/ATen/Backend.h Outdated

This comment was marked as off-topic.

Sign in to view

ezyang force-pushed the export-D9301861 branch from 9d197d4 to fa8b4dd Compare August 17, 2018 14:51

ezyang requested review from ebetica, pietern and teng-li as code owners August 17, 2018 14:51

ezyang force-pushed the export-D9301861 branch from fa8b4dd to 0083f19 Compare August 17, 2018 14:57

gchanan approved these changes Aug 17, 2018

View reviewed changes

ezyang force-pushed the export-D9301861 branch from 0083f19 to dfb9cc2 Compare August 17, 2018 16:22

ezyang force-pushed the export-D9301861 branch from dfb9cc2 to 8fcafd6 Compare August 17, 2018 17:21

ezyang force-pushed the export-D9301861 branch from 8fcafd6 to a1b2abb Compare August 17, 2018 18:02

ezyang force-pushed the export-D9301861 branch from a1b2abb to e07f334 Compare August 18, 2018 03:27

ezyang force-pushed the export-D9301861 branch from e07f334 to 33ea1b0 Compare August 18, 2018 03:29

ezyang force-pushed the export-D9301861 branch from 33ea1b0 to 68e7f38 Compare August 18, 2018 16:18

facebook-github-bot closed this in facebookresearch/pytext@ed2d43f Aug 19, 2018

avmgithub mentioned this pull request Aug 20, 2018

[ppc64le/pytorch] build break due to #10478 #10698

Closed

ezyang added the merged label Jun 26, 2019

Conversation

ezyang commented Aug 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

goldsborough left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

gchanan left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as off-topic.

Uh oh!

ezyang commented Aug 16, 2018

Uh oh!

gchanan left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ezyang commented Aug 13, 2018 •

edited

Loading