-
Notifications
You must be signed in to change notification settings - Fork 487
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixing CUDA delta issue #169
Conversation
attributions_orig = attr_orig.attribute(**kwargs) | ||
if attr_orig.has_convergence_delta(): | ||
attributions_orig = attr_orig.attribute( | ||
return_convergence_delta=True, **kwargs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Do we want to return attributions_orig, delta =
here ?
tests/attr/test_data_parallel.py
Outdated
if batch_size: | ||
attributions_dp = attr_dp.attribute( | ||
internal_batch_size=batch_size, **kwargs | ||
) | ||
else: | ||
attributions_dp = attr_dp.attribute(**kwargs) | ||
if attr_orig.has_convergence_delta(): | ||
attributions_dp = attr_dp.attribute( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: also here: attributions_dp, delta
?
Thanks, good point, updated! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome! LGTM! Thank you!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vivekmig is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Summary: Convergence delta was failing on GPUs since the total attribution tensor was not being created on the same device, this fixes that bug. Also, adds data parallel / CUDA tests for computing deltas whenever available. Issue: pytorch#163 Pull Request resolved: pytorch#169 Differential Revision: D18440711 Pulled By: vivekmig fbshipit-source-id: 5b67ba75492eb3c6933d13e1e67914bf1d3e5241
Summary: Convergence delta was failing on GPUs since the total attribution tensor was not being created on the same device, this fixes that bug. Also, adds data parallel / CUDA tests for computing deltas whenever available. Issue: pytorch#163 Pull Request resolved: pytorch#169 Differential Revision: D18440711 Pulled By: vivekmig fbshipit-source-id: 5b67ba75492eb3c6933d13e1e67914bf1d3e5241
Summary: Convergence delta was failing on GPUs since the total attribution tensor was not being created on the same device, this fixes that bug. Also, adds data parallel / CUDA tests for computing deltas whenever available. Issue: pytorch#163 Pull Request resolved: pytorch#169 Differential Revision: D18440711 Pulled By: vivekmig fbshipit-source-id: 5b67ba75492eb3c6933d13e1e67914bf1d3e5241
Summary: Convergence delta was failing on GPUs since the total attribution tensor was not being created on the same device, this fixes that bug. Also, adds data parallel / CUDA tests for computing deltas whenever available. Issue: pytorch#163 Pull Request resolved: pytorch#169 Differential Revision: D18440711 Pulled By: vivekmig fbshipit-source-id: 5b67ba75492eb3c6933d13e1e67914bf1d3e5241
Convergence delta was failing on GPUs since the total attribution tensor was not being created on the same device, this fixes that bug. Also, adds data parallel / CUDA tests for computing deltas whenever available.
Issue: #163