KEMBAR78
[ROCM] fix bug in #60313 by micmelesse · Pull Request #61073 · pytorch/pytorch · GitHub
Skip to content

[ROCM] fix bug in #60313 #61073

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 7 commits into from
Closed

[ROCM] fix bug in #60313 #61073

wants to merge 7 commits into from
+125 −28

Conversation

@micmelesse
Copy link
Contributor

@micmelesse micmelesse commented Jun 30, 2021 •
edited
Loading

This PR fixes a bug in #60313. Where the tensors generated by _generate_valid_rocfft_input are on the cpu instead of the gpu. This was due to using numpy to generate tensors and converting it to pytorch using torch.from_numpy. This leads to the generated tensors staying on the cpu. We now generate the tensors using pytorch itself which carries over the device type of the input tensors to the generated tensor.

@facebook-github-bot
Copy link
Contributor

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

💊 CI failures summary and remediations

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

  • 1/1 failures introduced in this PR

🕵️ 1 new failure recognized by patterns

The following CI failures do not appear to be due to upstream breakages:

See GitHub Actions build Windows CI (pytorch-win-vs2019-cuda10-cudnn7-py3) / test (default, 2, 2, windows.8xlarge.nvidia.gpu) (1/1)

Step: "Install Cuda" (full log | diagnosis details | 🔁 rerun)

2021-07-09T01:46:52.7093405Z ls: cannot access ...UDA/v10.1/bin/nvcc.exe': No such file or directory 
2021-07-09T01:46:51.3709010Z 
2021-07-09T01:46:51.3709476Z Folders: 11
2021-07-09T01:46:51.3709817Z Files: 130
2021-07-09T01:46:51.3710257Z Size:       907512
2021-07-09T01:46:51.3711016Z Compressed: 111420
2021-07-09T01:46:51.3770010Z + mkdir -p 'C:/Program Files/NVIDIA Corporation/NvToolsExt'
2021-07-09T01:46:51.4185990Z + cp -r NvToolsExt/bin NvToolsExt/docs NvToolsExt/include NvToolsExt/lib NvToolsExt/samples 'C:/Program Files/NVIDIA Corporation/NvToolsExt/'
2021-07-09T01:46:52.6904200Z + export 'NVTOOLSEXT_PATH=C:\Program Files\NVIDIA Corporation\NvToolsExt\'
2021-07-09T01:46:52.6905881Z + NVTOOLSEXT_PATH='C:\Program Files\NVIDIA Corporation\NvToolsExt\'
2021-07-09T01:46:52.6907325Z + ls '/c/Program Files/NVIDIA GPU Computing Toolkit/CUDA/v10.1/bin/nvcc.exe'
2021-07-09T01:46:52.7093405Z ls: cannot access '/c/Program Files/NVIDIA GPU Computing Toolkit/CUDA/v10.1/bin/nvcc.exe': No such file or directory
2021-07-09T01:46:52.7099154Z + echo 'CUDA installation failed'
2021-07-09T01:46:52.7100189Z + mkdir -p /c/w/build-results
2021-07-09T01:46:52.7101063Z CUDA installation failed
2021-07-09T01:46:52.7292416Z + 7z a 'c:\w\build-results\cuda_install_logs.7z' cuda_install_logs
2021-07-09T01:46:53.1640507Z 
2021-07-09T01:46:53.1641724Z 7-Zip 19.00 (x64) : Copyright (c) 1999-2018 Igor Pavlov : 2019-02-21
2021-07-09T01:46:53.1642317Z 
2021-07-09T01:46:53.1643051Z Scanning the drive:
2021-07-09T01:46:53.1643845Z 1 folder, 2 files, 4189346 bytes (4092 KiB)
2021-07-09T01:46:53.1644159Z

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.

@github-actions github-actions bot added the module: rocm AMD GPU support for Pytorch label Jun 30, 2021
@micmelesse micmelesse changed the title [ROCM] force tensor to cuda [ROCM] fix bug in #60313 Jul 9, 2021
@micmelesse micmelesse marked this pull request as ready for review July 9, 2021 17:38
@ezyang ezyang requested a review from malfet July 12, 2021 13:05
@ezyang ezyang added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Jul 12, 2021
malfet
malfet approved these changes Jul 12, 2021
View reviewed changes
Copy link
Contributor

@malfet malfet left a comment

Choose a reason for hiding this comment

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

This affects ROCm only, so I trust you know what you are doing.
But frankly, it feels like rather than documenting limitations of ROCfft support, PR is trying to avoid testing those corner cases

malfet
malfet reviewed Jul 12, 2021
View reviewed changes
test/test_spectral_ops.py
# rocFFT requires/assumes that the input to hipfftExecC2R or hipfftExecZ2D
# is of the form that is a valid output from a real to complex transform
# (i.e. it cannot be a set of random numbers)
# So for ROCm, call np.fft.rfftn and use its output as the input
Copy link
Contributor

Choose a reason for hiding this comment

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

This comment seems out of date, isn't it?

@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

@malfet merged this pull request in ac6ec0e.

@micmelesse micmelesse mentioned this pull request Nov 19, 2021
Closed
@micmelesse micmelesse mentioned this pull request Dec 8, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@malfet malfet malfet approved these changes

Assignees

No one assigned

Labels

cla signed Merged module: rocm AMD GPU support for Pytorch 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.

5 participants

@micmelesse @facebook-github-bot @malfet @ezyang @pytorchbot