Skip to content
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

Closed
wants to merge 4 commits into from
Closed

Conversation

vivekmig
Copy link
Contributor

@vivekmig vivekmig commented Nov 11, 2019

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

attributions_orig = attr_orig.attribute(**kwargs)
if attr_orig.has_convergence_delta():
attributions_orig = attr_orig.attribute(
return_convergence_delta=True, **kwargs
Copy link
Contributor

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 ?

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(
Copy link
Contributor

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 ?

@vivekmig
Copy link
Contributor Author

Thanks, good point, updated!

Copy link
Contributor

@NarineK NarineK left a 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!

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a 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.

@facebook-github-bot
Copy link
Contributor

@vivekmig merged this pull request in 5737de1.

miguelmartin75 pushed a commit to miguelmartin75/captum that referenced this pull request Nov 20, 2019
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
miguelmartin75 pushed a commit to miguelmartin75/captum that referenced this pull request Dec 20, 2019
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
miguelmartin75 pushed a commit to miguelmartin75/captum that referenced this pull request Dec 20, 2019
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
NarineK pushed a commit to NarineK/captum-1 that referenced this pull request Nov 19, 2020
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants