Make Context to be Device-agnostic Step by Step (1/N) #136519
Conversation
- make init to be device-agnostic and move it to AcceleratorHooksInterface - refactoring context related to device initialization [ghstack-poisoned]
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/136519
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 3d68218 with merge base 7f88bf9 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
- make init to be device-agnostic and move it to AcceleratorHooksInterface - refactoring context related to device initialization [ghstack-poisoned]
- make init to be device-agnostic and move it to AcceleratorHooksInterface - refactoring context related to device initialization [ghstack-poisoned]
- make init to be device-agnostic and move it to AcceleratorHooksInterface - refactoring context related to device initialization [ghstack-poisoned]
- make init to be device-agnostic and move it to AcceleratorHooksInterface - refactoring context related to device initialization [ghstack-poisoned]
|
@pytorchbot revert -m "breaking internal tests related to MITA, @ezyang has a forward fix?" -c ghfirst |
|
@pytorchbot successfully started a revert job. Check the current status here. |
This reverts commit a6eb020. Reverted #136526 on behalf of https://github.com/clee2000 due to breaking internal tests related to MITA, @ezyang has a forward fix? ([comment](#136519 (comment)))
This reverts commit 4a8e493. Reverted #136519 on behalf of https://github.com/clee2000 due to breaking internal tests related to MITA, @ezyang has a forward fix? ([comment](#136519 (comment)))
---- - add new method(getDefaultGenerator, getNewGenerator) into AcceleratorHooksInterface Pull Request resolved: #136526 Approved by: https://github.com/ezyang, https://github.com/EikanWang
|
@FFFrog your PR has been successfully reverted. |
|
I'm going to help, it's just one small problem now |
|
I'm going to attempt to fix this directly, but a way to fix this from first principles in OSS, is to not rename any of the virtual methods on the hooks classes, and have a new implementation of init() in the subclass interfaces we let people subclass from that call the old method by default. What is done in this PR is cleaner but it's BC breaking and concretely what I have to do is rename initMTIA to init internally. |
- make init to be device-agnostic and move it to AcceleratorHooksInterface - refactoring context related to device initialization ghstack-source-id: 3cb7e64 Pull Request resolved: pytorch/pytorch#136519
|
My principle is to make Context.h and DeviceInterface related files clearer and easier to understand, but the changes in the current pr do introduce BC breaking, which should not be the case for a widely used project like PyTorch.
I agree with your point of view, so I updated the PR and made the following changes:
|
|
It turns out that the BC fix is pretty easy. I'm going to land my own copy as I need to also land an fbcode change |
|
Internal diff with the BC update fixes https://www.internalfb.com/diff/D64471142 |
Summary: - make init to be device-agnostic and move it to AcceleratorHooksInterface - refactoring context related to device initialization Original pull request: pytorch#136519 Test Plan: contbuild & OSS CI, see https://hud.pytorch.org/commit/pyt 8000 orch/pytorch/4a8e49389c33934234dc89616fd17a58e760e2e7 Reviewed By: malfet Differential Revision: D64471142
) Summary: - make init to be device-agnostic and move it to AcceleratorHooksInterface - refactoring context related to device initialization Original pull request: #136519 Test Plan: contbuild & OSS CI, see https://hud.pytorch.org/commit/pytorch/pytorch/4a8e49389c33934234dc89616fd17a58e760e2e7 Reviewed By: malfet Differential Revision: D64471142 Pull Request resolved: #138155 Approved by: https://github.com/malfet, https://github.com/bobrenjc93
Stack from ghstack (oldest at bottom):