Skip to content

[ROCm] Disable MIOpen for empty tensors for RNN#117672

Closed
dnikolaev-amd wants to merge 1 commit intopytorch:mainfrom
ROCm:disable_miopen_for_empty_tensors_for_rnn
Closed

[ROCm] Disable MIOpen for empty tensors for RNN#117672
dnikolaev-amd wants to merge 1 commit intopytorch:mainfrom
ROCm:disable_miopen_for_empty_tensors_for_rnn

Conversation

@dnikolaev-amd
Copy link
Contributor

@dnikolaev-amd dnikolaev-amd commented Jan 17, 2024

Some MIOpen RNN functions (lstm, rnn, gru) can't work with empty tensors and return error "MIOpen Error: Lengths must be > 0"
This PR disables MIOpen tor empty tensors and force to use native methods
The solution is based on condition of using CUDNN

if (self.sym_numel() == 0) return false;

It also fix test_nn.py::TestNN::test_RNN_input_size_zero on ROCM

cc @jeffdaily @sunway513 @jithunnair-amd @pruthvistony @ROCmSupport @dllehr-amd @jataylo @hongxiayang

@pytorch-bot
Copy link

pytorch-bot bot commented Jan 17, 2024

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/117672

Note: Links to docs will display an error until the docs builds have been completed.

✅ You can merge normally! (2 Unrelated Failures)

As of commit ae14744 with merge base 79811e7 (image):

UNSTABLE - The following jobs failed but were likely due to flakiness present on trunk and has been marked as unstable:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@dnikolaev-amd dnikolaev-amd changed the title Disable MIOpen for emty tensors for RNN Disable MIOpen for empty tensors for RNN Jan 17, 2024
@dnikolaev-amd
Copy link
Contributor Author

@pytorchbot label ciflow/rocm

@pytorch-bot pytorch-bot bot added the ciflow/rocm Trigger "default" config CI on ROCm label Jan 17, 2024
@pytorch-bot
Copy link

pytorch-bot bot commented Jan 17, 2024

Please seek CI approval before scheduling CIFlow labels

@pytorch-bot pytorch-bot bot removed the ciflow/rocm Trigger "default" config CI on ROCm label Jan 17, 2024
@pruthvistony pruthvistony added the ciflow/rocm Trigger "default" config CI on ROCm label Jan 18, 2024
@jeffdaily jeffdaily changed the title Disable MIOpen for empty tensors for RNN [ROCm] Disable MIOpen for empty tensors for RNN Jan 18, 2024
@dnikolaev-amd dnikolaev-amd marked this pull request as ready for review January 18, 2024 17:17
@pytorch-bot pytorch-bot bot added the module: rocm AMD GPU support for Pytorch label Jan 18, 2024
@jeffdaily
Copy link
Collaborator

@pytorchbot rebase

@pytorchmergebot
Copy link
Collaborator

@pytorchbot started a rebase job onto refs/remotes/origin/viable/strict. Check the current status here

@pytorchmergebot
Copy link
Collaborator

Successfully rebased disable_miopen_for_empty_tensors_for_rnn onto refs/remotes/origin/viable/strict, please pull locally before adding more changes (for example, via git checkout disable_miopen_for_empty_tensors_for_rnn && git pull --rebase)

@pytorchmergebot pytorchmergebot force-pushed the disable_miopen_for_empty_tensors_for_rnn branch from d7bb0f6 to ae14744 Compare January 18, 2024 17:21
@cpuhrsch cpuhrsch added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Jan 20, 2024
@jeffdaily jeffdaily added the ciflow/trunk Trigger trunk jobs on your pull request label Jan 22, 2024
@jataylo
Copy link
Collaborator

jataylo commented Jan 23, 2024

@pytorchbot merge -f "Two unstable suites failing"

@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes). Please use -f as last resort and instead consider -i/--ignore-current to continue the merge ignoring current failures. This will allow currently pending tests to finish and report signal before the merge.

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

@pruthvistony pruthvistony deleted the disable_miopen_for_empty_tensors_for_rnn branch January 24, 2024 18:59
BLOrange-AMD pushed a commit to ROCm/pytorch that referenced this pull request Mar 1, 2024
Some MIOpen RNN functions (lstm, rnn, gru) can't work with empty tensors and return error "MIOpen Error: Lengths must be > 0"
This PR disables MIOpen tor empty tensors and force to use native methods
The solution is based on condition of using CUDNN https://github.com/pytorch/pytorch/blob/3a52147cc59b240737602d3d046080bbf6f567f1/aten/src/ATen/native/TensorProperties.cpp#L91
It also fix [test_nn.py::TestNN::test_RNN_input_size_zero](https://github.com/pytorch/pytorch/blob/29fa6fbc4eda6c02ecdfd73b74a8702187c4fc44/test/test_nn.py#L4592) on ROCM

Pull Request resolved: pytorch#117672
Approved by: https://github.com/cpuhrsch
dnikolaev-amd added a commit to ROCm/pytorch that referenced this pull request Apr 8, 2024
Some MIOpen RNN functions (lstm, rnn, gru) can't work with empty tensors and return error "MIOpen Error: Lengths must be > 0"
This PR disables MIOpen tor empty tensors and force to use native methods
The solution is based on condition of using CUDNN https://github.com/pytorch/pytorch/blob/3a52147cc59b240737602d3d046080bbf6f567f1/aten/src/ATen/native/TensorProperties.cpp#L91
It also fix [test_nn.py::TestNN::test_RNN_input_size_zero](https://github.com/pytorch/pytorch/blob/29fa6fbc4eda6c02ecdfd73b74a8702187c4fc44/test/test_nn.py#L4592) on ROCM

Pull Request resolved: pytorch#117672
Approved by: https://github.com/cpuhrsch
pruthvistony pushed a commit to ROCm/pytorch that referenced this pull request Apr 8, 2024
Some MIOpen RNN functions (lstm, rnn, gru) can't work with empty tensors and return error "MIOpen Error: Lengths must be > 0"
This PR disables MIOpen tor empty tensors and force to use native methods
The solution is based on condition of using CUDNN https://github.com/pytorch/pytorch/blob/3a52147cc59b240737602d3d046080bbf6f567f1/aten/src/ATen/native/TensorProperties.cpp#L91
It also fix [test_nn.py::TestNN::test_RNN_input_size_zero](https://github.com/pytorch/pytorch/blob/29fa6fbc4eda6c02ecdfd73b74a8702187c4fc44/test/test_nn.py#L4592) on ROCM

Pull Request resolved: pytorch#117672
Approved by: https://github.com/cpuhrsch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ciflow/rocm Trigger "default" config CI on ROCm ciflow/trunk Trigger trunk jobs on your pull request 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

Development

Successfully merging this pull request may close these issues.

7 participants