Skip to content

[CUDA10 fixes] Adds max plan number for CUDA 10 cufft plan cache array#12553

Closed
syed-ahmed wants to merge 2 commits intopytorch:masterfrom
syed-ahmed:cufft-max-plan-fix
Closed

[CUDA10 fixes] Adds max plan number for CUDA 10 cufft plan cache array#12553
syed-ahmed wants to merge 2 commits intopytorch:masterfrom
syed-ahmed:cufft-max-plan-fix

Conversation

@syed-ahmed
Copy link
Collaborator

@ssnl As per your review in #12017, I added a max plan number for CUDA 10 path. Our internal cuFFT team couldn't suggest a number since the limit depends on host/device memory. That is, a plan allocates some buffers on the device and also creates objects for the plans on the host side. I raised this number to 4x arbitrarily per you suggestion.

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.

@@ -394,12 +393,12 @@ class CuFFTParamsLRUCache {
// in CUDA 10. Hence, when compiling with CUDA 10, just
// don't do the erase.
#if CUDA_VERSION < 10000

This comment was marked as off-topic.

for (size_t i = 0; i < cur_size - _max_size; i++) {
delete_it--;
_cache_map.erase(delete_it->first);
auto cur_size = _usage_list.size();

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

Copy link
Collaborator

@ssnl ssnl left a comment

Choose a reason for hiding this comment

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

The max capacity is not enforced, but it should be, considering that cuFFT plans may use GPU memory.

@syed-ahmed
Copy link
Collaborator Author

@ssnl Thanks for the review and explanation! I have pushed the requested changes.

@ssnl
Copy link
Collaborator

ssnl commented Oct 11, 2018

Thanks!

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.

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

zdevito pushed a commit to zdevito/ATen that referenced this pull request Oct 12, 2018
Summary:
SsnL As per your review in pytorch/pytorch#12017, I added a max plan number for CUDA 10 path. Our internal cuFFT team couldn't suggest a number since the limit depends on host/device memory. That is, a plan allocates some buffers on the device and also creates objects for the plans on the host side. I raised this number to 4x arbitrarily per you suggestion.
Pull Request resolved: pytorch/pytorch#12553

Differential Revision: D10320832

Pulled By: SsnL

fbshipit-source-id: 3148d45cd280dffb2039756e2f6a74fbc7aa086d
facebook-github-bot pushed a commit that referenced this pull request Oct 16, 2018
Summary:
Follow up of #12553
Pull Request resolved: #12665

Differential Revision: D10385615

Pulled By: SsnL

fbshipit-source-id: 44fe9ec75cb735de37c56270f160a16a1d2bfb64
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants