Skip to content

Conversation

lw
Copy link
Contributor

@lw lw commented Apr 27, 2021

Stack from ghstack:

Make CUDAFuture autodetect the devicetype from its arguments (which thus change from DeviceIndices to full Devices). This in fact transforms CUDAFuture into a AnythingFuture, since it's not tied to CUDA in any way anymore. Having made it fully device-agnostic, we'll merge it into ivalue::Future in the next PR.

Differential Revision: D28032711

Make CUDAFuture autodetect the devicetype from its arguments (which thus change from DeviceIndices to full Devices). This in fact transforms CUDAFuture into a AnythingFuture, since it's not tied to CUDA in any way anymore. Having made it fully device-agnostic, we'll merge it into ivalue::Future in the next PR.

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

[ghstack-poisoned]
@facebook-github-bot facebook-github-bot added oncall: jit Add this issue/PR to JIT oncall triage queue cla signed oncall: distributed Add this issue/PR to distributed oncall triage queue labels Apr 27, 2021
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Apr 27, 2021

💊 CI failures summary and remediations

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


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

1 failure not recognized by patterns:

Job Step Action
CircleCI binary_linux_libtorch_3_7m_cpu_devtoolset7_shared-with-deps_build Build 🔁 rerun

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.

Click here to manually regenerate this comment.

lw added 4 commits April 27, 2021 14:52
Make CUDAFuture autodetect the devicetype from its arguments (which thus change from DeviceIndices to full Devices). This in fact transforms CUDAFuture into a AnythingFuture, since it's not tied to CUDA in any way anymore. Having made it fully device-agnostic, we'll merge it into ivalue::Future in the next PR.

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

[ghstack-poisoned]
Make CUDAFuture autodetect the devicetype from its arguments (which thus change from DeviceIndices to full Devices). This in fact transforms CUDAFuture into a AnythingFuture, since it's not tied to CUDA in any way anymore. Having made it fully device-agnostic, we'll merge it into ivalue::Future in the next PR.

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

[ghstack-poisoned]
Make CUDAFuture autodetect the devicetype from its arguments (which thus change from DeviceIndices to full Devices). This in fact transforms CUDAFuture into a AnythingFuture, since it's not tied to CUDA in any way anymore. Having made it fully device-agnostic, we'll merge it into ivalue::Future in the next PR.

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

[ghstack-poisoned]
Make CUDAFuture autodetect the devicetype from its arguments (which thus change from DeviceIndices to full Devices). This in fact transforms CUDAFuture into a AnythingFuture, since it's not tied to CUDA in any way anymore. Having made it fully device-agnostic, we'll merge it into ivalue::Future in the next PR.

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

[ghstack-poisoned]
@@ -34,13 +34,7 @@ def __init__(self, *, devices: Optional[List[Union[int, str, torch.device]]] = N
"""
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume docs for devices arg will be added in follow PRs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I mentioned it in the paragraph but didn't add a specific section for it, let me do that.

impl_(c10::impl::getDeviceGuardImpl(c10::kCUDA)),
devices_(sortDevices(std::move(devices))) {
impl_(getImplForDevices(devices)),
devices_(getSortedIndicesOfDevices(impl_, devices)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

curious, any reason for not just storing devices instead of device indices in this change??

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At this stage I wanted to change as few things as possible to keep the delta small and thus help see that this is not changing behaviors. But, yes, I was thinking the same and I'll probably do a follow-up "polishing" PR.

lw added 3 commits April 28, 2021 12:18
Make CUDAFuture autodetect the devicetype from its arguments (which thus change from DeviceIndices to full Devices). This in fact transforms CUDAFuture into a AnythingFuture, since it's not tied to CUDA in any way anymore. Having made it fully device-agnostic, we'll merge it into ivalue::Future in the next PR.

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

[ghstack-poisoned]
Make CUDAFuture autodetect the devicetype from its arguments (which thus change from DeviceIndices to full Devices). This in fact transforms CUDAFuture into a AnythingFuture, since it's not tied to CUDA in any way anymore. Having made it fully device-agnostic, we'll merge it into ivalue::Future in the next PR.

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

[ghstack-poisoned]
Make CUDAFuture autodetect the devicetype from its arguments (which thus change from DeviceIndices to full Devices). This in fact transforms CUDAFuture into a AnythingFuture, since it's not tied to CUDA in any way anymore. Having made it fully device-agnostic, we'll merge it into ivalue::Future in the next PR.

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

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

This pull request has been merged in 71c2f88.

@facebook-github-bot facebook-github-bot deleted the gh/lw/140/head branch May 3, 2021 14:16
krshrimali pushed a commit to krshrimali/pytorch that referenced this pull request May 19, 2021
Summary:
Pull Request resolved: pytorch#57051

Make CUDAFuture autodetect the devicetype from its arguments (which thus change from DeviceIndices to full Devices). This in fact transforms CUDAFuture into a AnythingFuture, since it's not tied to CUDA in any way anymore. Having made it fully device-agnostic, we'll merge it into ivalue::Future in the next PR.
ghstack-source-id: 127713134

(Note: this ignores all push blocking failures!)

Test Plan: CI

Reviewed By: mrshenli

Differential Revision: D28032711

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

Successfully merging this pull request may close these issues.

3 participants