Open ezyang opened 4 days ago
This pull request was exported from Phabricator. Differential Revision: D52890934
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 95.70%. Comparing base (
0cab5d2
) to head (7ea9e15
).
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Summary: As laid out in https://github.com/pytorch/pytorch/issues/81462#issuecomment-1838731223 the change in https://github.com/pytorch/pytorch/pull/104321 was not necessary and largely destroys the type-safety of
torch.nn.Module
instances.As far as I can see, the underlying issue of https://github.com/pytorch/pytorch/issues/81462 in
torch.nn.parallel.DistributedDataParallel
has been fixed in the meantime by actually typingregister_comm_hook
correctly.The proper solution to issues like https://github.com/pytorch/pytorch/issues/81462 is to give the underlying field/method a proper type annotation, then there should be no need to go for a "type system disabling
__getattr__
".(I'll probably be offline for a while, not able to react here...)
cc H-Huang awgu kwen2501 wanchaol fegin fduwjj wz337 wconstab d4l3k c-p-i-o voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx ipiszy yf225 chenyang78 kadeng muchulee8 ColinPeppler amjames desertfire chauhang aakhundov avikchaudhuri gmagogsfm zhxchen17 tugsbayasgalan angelayi suo ydwu4 XilunWu rec mrshenli pritamdamania87 zhaojuanmao satgera rohan-varma gqchen aazzolini osalpekar jiayisuse tianyu-l kiukchung lucasllc
X-link: https://github.com/pytorch/pytorch/pull/115074
Differential Revision: D52890934
Pulled By: ezyang