KEMBAR78
Analysing time taken by gradgrad checks for Spectral Functions by krshrimali · Pull Request #60435 · pytorch/pytorch · GitHub
Skip to content

Analysing time taken by gradgrad checks for Spectral Functions #60435

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
wants to merge 4 commits into from
Closed

Analysing time taken by gradgrad checks for Spectral Functions #60435

wants to merge 4 commits into from

Conversation

@krshrimali
Copy link
Contributor

@krshrimali krshrimali commented Jun 22, 2021 •
edited
Loading

Description: SpectralFuncInfo defines decorator mentioning: "gradgrad is quite slow". This PR re-analyses that statement since things have changed with gradient tests.

Test times: #60435 (comment)

Follow-up of #57802

cc: @mruberry

@krshrimali krshrimali added the module: testing Issues related to the torch.testing module (not tests) label Jun 22, 2021
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Jun 22, 2021 •
edited
Loading

💊 CI failures summary and remediations

As of commit ff0d192 (more details on the Dr. CI page and at hud.pytorch.org/pr/60435):

💚 💚 Looks good so far! There are no failures yet. 💚 💚

Preview docs built from this PR

This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

@krshrimali
Copy link
Contributor Author

krshrimali commented Jun 22, 2021 •
edited
Loading

Note: Times are in seconds. There are few functions which don't test for complex128, and hence the - in those cells. Spectral Functions (currently): 'fft.fft', 'fft.fftn', 'fft.hfft', 'fft.ifft', 'fft.ifftn', 'fft.ihfft', 'fft.irfft', 'fft.irfftn', 'fft.rfft', 'fft.rfftn'

Function Name Time Taken (complex128) Time Taken (float64)
fft.fft 0.562 0.251
fft.fftn 0.647 0.267
fft.hfft 0.521 0.224
fft.ifft 0.579 0.260
fft.ifftn 0.659 0.270
fft.ihfft - 0.264
fft.irfft 0.521 0.234
fft.irfftn 0.594 0.238
fft.rfft - 0.234
fft.rfftn - 0.249

Test Log: (CI Link: https://app.circleci.com/pipelines/github/pytorch/pytorch/339893/workflows/5b2b5f3f-c2c9-45d8-b325-7a1b7d3b39b4/jobs/14291550)

 Jun 22 06:23:13   test_fn_gradgrad_fft_fft_cuda_complex128 (__main__.TestGradientsCUDA) ... ok (0.562s)
 Jun 22 06:23:13   test_fn_gradgrad_fft_fft_cuda_float64 (__main__.TestGradientsCUDA) ... ok (0.251s)
 Jun 22 06:23:14   test_fn_gradgrad_fft_fftn_cuda_complex128 (__main__.TestGradientsCUDA) ... ok (0.647s)
 Jun 22 06:23:14   test_fn_gradgrad_fft_fftn_cuda_float64 (__main__.TestGradientsCUDA) ... ok (0.267s)
 Jun 22 06:23:15   test_fn_gradgrad_fft_hfft_cuda_complex128 (__main__.TestGradientsCUDA) ... ok (0.521s)
 Jun 22 06:23:15   test_fn_gradgrad_fft_hfft_cuda_float64 (__main__.TestGradientsCUDA) ... ok (0.224s)
 Jun 22 06:23:15   test_fn_gradgrad_fft_ifft_cuda_complex128 (__main__.TestGradientsCUDA) ... ok (0.579s)
 Jun 22 06:23:16   test_fn_gradgrad_fft_ifft_cuda_float64 (__main__.TestGradientsCUDA) ... ok (0.260s)
 Jun 22 06:23:16   test_fn_gradgrad_fft_ifftn_cuda_complex128 (__main__.TestGradientsCUDA) ... ok (0.659s)
 Jun 22 06:23:17   test_fn_gradgrad_fft_ifftn_cuda_float64 (__main__.TestGradientsCUDA) ... ok (0.270s)
 Jun 22 06:23:17   test_fn_gradgrad_fft_ihfft_cuda_float64 (__main__.TestGradientsCUDA) ... ok (0.264s)
 Jun 22 06:23:17   test_fn_gradgrad_fft_irfft_cuda_complex128 (__main__.TestGradientsCUDA) ... ok (0.521s)
 Jun 22 06:23:18   test_fn_gradgrad_fft_irfft_cuda_float64 (__main__.TestGradientsCUDA) ... ok (0.234s)
 Jun 22 06:23:18   test_fn_gradgrad_fft_irfftn_cuda_complex128 (__main__.TestGradientsCUDA) ... ok (0.594s)
 Jun 22 06:23:19   test_fn_gradgrad_fft_irfftn_cuda_float64 (__main__.TestGradientsCUDA) ... ok (0.238s)
 Jun 22 06:23:19   test_fn_gradgrad_fft_rfft_cuda_float64 (__main__.TestGradientsCUDA) ... ok (0.234s)
 Jun 22 06:23:19   test_fn_gradgrad_fft_rfftn_cuda_float64 (__main__.TestGradientsCUDA) ... ok (0.249s)

Looks like it's reasonable to delete the decorator from SpectralFuncInfo, all the tests take < 0.7 seconds on the CI. What do you think? @mruberry

@krshrimali krshrimali requested a review from mruberry June 22, 2021 07:13
@krshrimali krshrimali changed the title [DO NOT MERGE] Analysing time taken by gradgrad checks for Spectral Functions Analysing time taken by gradgrad checks for Spectral Functions Jun 22, 2021
@codecov
Copy link

codecov bot commented Jun 22, 2021 •
edited
Loading

Codecov Report

Merging #60435 (ff0d192) into master (15dc320) will decrease coverage by 0.00%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #60435      +/-   ##
==========================================
- Coverage   76.23%   76.23%   -0.01%     
==========================================
  Files        2054     2054              
  Lines      205033   205033              
==========================================
- Hits       156309   156301       -8     
- Misses      48724    48732       +8

@mrshenli mrshenli added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Jun 23, 2021
@mruberry
Copy link
Collaborator

I agree these all seem faster enough that we can run them. @albanD what do you think?

@albanD
Copy link
Collaborator

albanD commented Jun 24, 2021

Yes looks good! More testing that takes no time is always betterr!!

@krshrimali
Copy link
Contributor Author

Gentle ping @mruberry - please take a look whenever you find time. :)

mruberry
mruberry approved these changes Jul 15, 2021
View reviewed changes
Copy link
Collaborator

@mruberry mruberry left a comment

Choose a reason for hiding this comment

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

Cool, thanks @krshrimali!

@facebook-github-bot
Copy link
Contributor

@mruberry has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@mruberry merged this pull request in 0ea29a6.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mruberry mruberry mruberry approved these changes

Assignees

No one assigned

Labels

cla signed Merged module: testing Issues related to the torch.testing module (not tests) open source triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@krshrimali @facebook-github-bot @mruberry @albanD @mrshenli @pytorchbot