pytorch / captum

Model interpretability and understanding for PyTorch
https://captum.ai
BSD 3-Clause "New" or "Revised" License
4.94k stars 499 forks source link

Fix type-safety of `torch.nn.Module` instances #1446

Open ezyang opened 4 days ago

ezyang commented 4 days ago

Summary: X-link: https://github.com/ctrl-labs/src2/pull/38809

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 typing register_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

facebook-github-bot commented 4 days ago

This pull request was exported from Phabricator. Differential Revision: D52890934