Skip to content

[ROCm] [BUGFIX] Re-enable rocm-specific tuning parameters#130617

Closed
jataylo wants to merge 1 commit intopytorch:mainfrom
jataylo:rocm-inductor-tuning-knobs-fix
Closed

[ROCm] [BUGFIX] Re-enable rocm-specific tuning parameters#130617
jataylo wants to merge 1 commit intopytorch:mainfrom
jataylo:rocm-inductor-tuning-knobs-fix

Conversation

@jataylo
Copy link
Collaborator

@jataylo jataylo commented Jul 12, 2024

Small bug fix - #124592 replaced the torch.version.hip with device_props but made a mistake in porting the original logic.

The original code was:

if torch.version.hip is not None:

Which was incorrectly replaced by:

if self.device_props.type != "hip":

Perhaps we need to write some unit tests here in the future.

cc @jeffdaily @sunway513 @jithunnair-amd @pruthvistony @ROCmSupport @dllehr-amd @hongxiayang @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @peterbell10 @ipiszy @yf225 @chenyang78 @kadeng @muchulee8 @ColinPeppler @amjames @desertfire @chauhang

@jataylo jataylo requested review from jansel, malfet and masnesral July 12, 2024 13:50
@pytorch-bot
Copy link

pytorch-bot bot commented Jul 12, 2024

🔗 Helpful Links

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

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

✅ No Failures

As of commit d0c4c44 with merge base da030e7 (image):
💚 Looks good so far! There are no failures yet. 💚

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

@pytorch-bot pytorch-bot bot added ciflow/inductor ciflow/rocm Trigger "default" config CI on ROCm module: inductor module: rocm AMD GPU support for Pytorch labels Jul 12, 2024
@jataylo jataylo added rocm priority high priority ROCm PRs from performance or other aspects topic: bug fixes topic category labels Jul 12, 2024
@jataylo jataylo requested a review from peterbell10 July 12, 2024 13:53
@jataylo
Copy link
Collaborator Author

jataylo commented Jul 12, 2024

@pytorchbot merge

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Jul 12, 2024
@pytorchmergebot
Copy link
Collaborator

Merge failed

Reason: This PR needs a release notes: label
If your changes are user facing and intended to be a part of release notes, please use a label starting with release notes:.

If not, please add the topic: not user facing label.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "topic: not user facing"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

Details for Dev Infra team Raised by workflow job

@jataylo
Copy link
Collaborator Author

jataylo commented Jul 12, 2024

@pytorchbot merge

@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

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

xuhancn pushed a commit to xuhancn/pytorch that referenced this pull request Jul 25, 2024
…0617)

Small bug fix - pytorch#124592 replaced the torch.version.hip with device_props but made a mistake in porting the original logic.

The original code was:
```
if torch.version.hip is not None:
```

Which was incorrectly replaced by:
```
if self.device_props.type != "hip":
```

Perhaps we need to write some unit tests here in the future.

Pull Request resolved: pytorch#130617
Approved by: https://github.com/masnesral
pytorchmergebot pushed a commit that referenced this pull request Sep 11, 2024
Small bug fix - #124592 replaced the torch.version.hip with device_props but made a mistake in porting the original logic.

The original code was:
`if torch.version.hip is not None:`

Which was incorrectly replaced by:
`if self.device_props.type != "hip":`

Another occurence of #130617

Pull Request resolved: #133852
Approved by: https://github.com/masnesral, https://github.com/malfet
jataylo added a commit to jataylo/pytorch that referenced this pull request Sep 16, 2024
…#133852)

Small bug fix - pytorch#124592 replaced the torch.version.hip with device_props but made a mistake in porting the original logic.

The original code was:
`if torch.version.hip is not None:`

Which was incorrectly replaced by:
`if self.device_props.type != "hip":`

Another occurence of pytorch#130617

Pull Request resolved: pytorch#133852
Approved by: https://github.com/masnesral, https://github.com/malfet

(cherry picked from commit da587de)
Chao1Han pushed a commit to Chao1Han/pytorch that referenced this pull request Sep 20, 2024
…#133852)

Small bug fix - pytorch#124592 replaced the torch.version.hip with device_props but made a mistake in porting the original logic.

The original code was:
`if torch.version.hip is not None:`

Which was incorrectly replaced by:
`if self.device_props.type != "hip":`

Another occurence of pytorch#130617

Pull Request resolved: pytorch#133852
Approved by: https://github.com/masnesral, https://github.com/malfet
kit1980 pushed a commit that referenced this pull request Sep 25, 2024
#136139)

Small bug fix - #124592 replaced the torch.version.hip with device_props but made a mistake in porting the original logic.

The original code was:
`if torch.version.hip is not None:`

Which was incorrectly replaced by:
`if self.device_props.type != "hip":`

Another occurence of #130617

Pull Request resolved: #133852
Approved by: https://github.com/masnesral, https://github.com/malfet

(cherry picked from commit da587de)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ciflow/inductor ciflow/rocm Trigger "default" config CI on ROCm ciflow/trunk Trigger trunk jobs on your pull request Merged module: inductor module: rocm AMD GPU support for Pytorch open source rocm priority high priority ROCm PRs from performance or other aspects topic: bug fixes topic category topic: not user facing topic category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants