Skip to content
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.

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

Disable MIOpen build from source for PyTorch wheels #16

Merged
merged 3 commits into from
Dec 1, 2022

Conversation

jithunnair-amd
Copy link

@jithunnair-amd jithunnair-amd commented Nov 1, 2022

Avoid rebuilding MIOpen from source for PyTorch wheels, and instead use the MIOpen .so that comes with the ROCm release. Bundle the .db files in the wheel instead of embedding them into the MIOpen .so.

BONUS: This should enable file I/O for the MIOpen .so, which can then read .kdb files if they're placed in the correct path (ie. ../share (wrt the location of the libMIOpen.so) by the user. Just to clarify, we will NOT bundle the .kdb files with the wheels due to their large size, so the user will need to manually install them and place them in the correct path above (we could provide the user a helper script for the same purpose).

FUTURE TODO:

  1. Helper script to install the .kdb files in the correct location for the wheels.
  2. Builder script to install miopen binaries from a non-standard location if needed.

Fixes: https://ontrack-internal.amd.com/browse/ROCMOPS-3381

@jithunnair-amd
Copy link
Author

jithunnair-amd commented Nov 2, 2022

This successfully produced PyTorch wheels: http://rocmhead.amd.com:8080/job/pytorch/job/manylinux_rocm_wheels/167/

However, micro benchmarking resnet50 runs 8% slower in the wheels using stock ROCm5.4 #23 MIOpen vs embedded MIOpen (http://rocmhead.amd.com:8080/job/pytorch/job/manylinux_rocm_wheels/164/). This could be because we use the release/rocm-rel-5.4-staging branch of MIOpen for the embedded MIOpen build, vs release/rocm-rel-5.4 branch for stock ROCm5.4 #23 MIOpen.

EDIT: After offline discussions, the conclusion is that it is expected that the rel-staging branch would have additional changes compared to the rel branch during the ROCm release cycle, as the rel-staging branch is used to stage changes that make it into the rel branch. However, post-release, any further tuning/bug fix updates will need to be released in a new ROCm (point?) release, or new MIOpen binaries at least.

We might need to add logic to the builder scripts to install these binaries over the stock MIOpen in case we wish to pick up the fixes/updates in the wheel at any point.

@atamazov
Copy link

atamazov commented Mar 1, 2023

/cc @sunway513 @JehandadKhan

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants