Skip to content

fix ZeroDivisionError in utils.bottleneck #11987

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.

Sign up for GitHub

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

Jump to bottom
Closed
egg-west wants to merge 4 commits into from
Closed

fix ZeroDivisionError in utils.bottleneck #11987

egg-west wants to merge 4 commits into from

Conversation

@egg-west
Copy link
Contributor

@egg-west egg-west commented Sep 23, 2018

ZeroDivisionError occurs when cuda_prof_exec_time is small enough.
This situation is normal for a project that has little CUDA work.

Or someone does not make his work transferred to CUDA successfully. In this time he profiles the code, this error occurs.

soumith
soumith reviewed Sep 25, 2018
View reviewed changes
torch/utils/bottleneck/__main__.py Outdated

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

apaszke
apaszke reviewed Sep 25, 2018
View reviewed changes
torch/utils/bottleneck/__main__.py Outdated

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

torch/utils/bottleneck/__main__.py Outdated

This comment was marked as off-topic.

Sign in to view

apaszke
apaszke reviewed Sep 28, 2018
View reviewed changes
torch/utils/bottleneck/__main__.py Outdated

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

@zou3519 zou3519 self-assigned this Oct 15, 2018
zou3519
zou3519 previously requested changes Oct 16, 2018
View reviewed changes
Copy link
Contributor

@zou3519 zou3519 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please change the check to check if the profiling result is empty. Thank you for noticing the problem and submitting a fix, @egg-west!

torch/utils/bottleneck/__main__.py Outdated

This comment was marked as off-topic.

Sign in to view

torch/utils/bottleneck/__main__.py Outdated

This comment was marked as off-topic.

Sign in to view

zou3519
zou3519 approved these changes Oct 17, 2018
View reviewed changes
Copy link
Contributor

@zou3519 zou3519 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix the lint, other than that, lgtm. Thanks @egg-west!

@egg-west
Copy link
Contributor Author

egg-west commented Oct 17, 2018

@apaszke PTAL.

@zou3519
Copy link
Contributor

zou3519 commented Oct 18, 2018

@egg-west could you rebase this on top of master so that the circle-ci tests run?

You can do that with:

git pull --rebase upstream master
git push -f

@egg-west
fix ZeroDivisionError
73a09d6 
ZeroDivisionError occurs when cuda_prof_exec_time is small enough.
This situation is normal for a project that has little CUDA work. 

Or someone does not make his work transferred to CUDA successfully. In this time he profiles the code, this error occurs.
@egg-west
edit the bound and add a bracket
d283434 
take the advice from advice and fix a newly discovered problem.
@egg-west
change the check in line 225
c8ca34c 
Change the check of `cuda_prof_exec_time` to check the length of cpu profile events.
@egg-west
update __main__.py fix lint
1144eaa 
line 230: remove space for blank line.
@egg-west
Copy link
Contributor Author

egg-west commented Oct 21, 2018

@zou3519 Thank you for all the help. I have done rebase, but there are still 2 test failed.

facebook-github-bot
facebook-github-bot reviewed Oct 22, 2018
View reviewed changes
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.

soumith is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@egg-west egg-west deleted the add-linjie branch October 31, 2018 07:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@facebook-github-bot facebook-github-bot facebook-github-bot left review comments

@zou3519 zou3519 zou3519 approved these changes

@colesbury colesbury Awaiting requested review from colesbury

@ezyang ezyang Awaiting requested review from ezyang

@gchanan gchanan Awaiting requested review from gchanan

@zdevito zdevito Awaiting requested review from zdevito

+2 more reviewers

@soumith soumith soumith approved these changes

@apaszke apaszke apaszke approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@zou3519 zou3519

Labels

open source

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@egg-west @zou3519 @soumith @apaszke @facebook-github-bot @ezyang