Hi @bmedishe!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

🔗 Helpful links

• 🧪  See artifacts and rendered test results at hud.pytorch.org/pr/74704
• 📄  Preview Python docs built from this PR
• 📄  Preview C++ docs built from this PR
• ❓Need help or want to give feedback on the CI? Visit our office hours

✅ No Failures (0 Pending)

As of commit 2765b91 (more details on the Dr. CI page):

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

@malfet I see that you imported this pull request , could you kindly confirm if this PR could be merged ?

@bmedishe not, as it still breaks internal build system with

    from .hipify import hipify_python  # type: ignore[import]
ModuleNotFoundError: No module named 'torch.utils.hipify'

Which raises a very valid question: aren't interface provided via torch.utils.hipify is part of public API and must be preserved going forward (even if implementation is now provided elsewhere)

@jithunnair-amd @pruthvistony

cc @shintaro-iwasaki Can you please take a look?

@shintaro-iwasaki has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@pytorchbot merge

(Initiating merge automatically since Phabricator Diff has merged)

@pytorchbot successfully started a merge job. Check the current status here

@bmedishe your PR has been successfully merged.

Hey @bmedishe.
You've committed this PR, but it does not have both a 'release notes: ...' and 'topics: ...' label. Please add one of each to the PR. The 'release notes: ...' label should represent the part of PyTorch that this PR changes (fx, autograd, distributed, etc) and the 'topics: ...' label should represent the kind of PR it is (not user facing, new feature, bug fix, perf improvement, etc). The list of valid labels can be found here for the 'release notes: ...' and here for the 'topics: ...'.
For changes that are 'topic: not user facing' there is no need for a release notes label.

@malfet I can't build torchvision after this PR, got

Traceback (most recent call last):
  File "setup.py", line 12, in <module>
      from torch.utils.cpp_extension import BuildExtension, CppExtension, CUDAExtension, CUDA_HOME
    File "/opt/pytorch/pytorch/torch/utils/cpp_extension.py", line 19, in <module>
      from .hipify import hipify_python  # type: ignore[import]
  ModuleNotFoundError: No module named 'torch.utils.hipify'

63c0bcf
pytorch/vision@28557e0

@pytorchbot revert -m "broke torchvision" -c weird

@pytorchbot successfully started a revert job. Check the current status here

@bmedishe your PR has been successfully reverted.

Maybe it was reflected to the GitHub side. I am sorry.

@jithunnair-amd when you have a chance to create a PR next time, could add the following in cpp_extenstion.py?

According to @xwang233's report, the following should address the issue.

try:
    from .hipify import hipify_python  # type: ignore[import]
    from .hipify.hipify_python import GeneratedFileCleaner  # type: ignore[import]
except ModuleNotFoundError:
    hipify_python = None  # type: ignore[assignment]
    GeneratedFileCleaner = None  # type: ignore[assignment]

Maybe it was reflected to the GitHub side. I am sorry.

@jithunnair-amd when you have a chance to create a PR next time, could add the following in cpp_extenstion.py?

According to @xwang233's report, the following should address the issue.

try:
    from .hipify import hipify_python  # type: ignore[import]
    from .hipify.hipify_python import GeneratedFileCleaner  # type: ignore[import]
except ModuleNotFoundError:
    hipify_python = None  # type: ignore[assignment]
    GeneratedFileCleaner = None  # type: ignore[assignment]

@shintaro-iwasaki I pushed the above suggested change

@malfet I can't build torchvision after this PR, got

Traceback (most recent call last):
  File "setup.py", line 12, in <module>
      from torch.utils.cpp_extension import BuildExtension, CppExtension, CUDAExtension, CUDA_HOME
    File "/opt/pytorch/pytorch/torch/utils/cpp_extension.py", line 19, in <module>
      from .hipify import hipify_python  # type: ignore[import]
  ModuleNotFoundError: No module named 'torch.utils.hipify'

63c0bcf pytorch/vision@28557e0

@xwang233 could you kindly mention reproduction steps for this error ? I could build torchvision after building pytorch from the above branch

@shintaro-iwasaki @xwang233 I think the key here is that the way you are building torchvision, it is trying to pick up pytorch from the source code path (File "/opt/pytorch/pytorch/torch/utils/cpp_extension.py"), instead of the install path. In this PR, we have ensured that torch.utils.hipify continues to be available in the PyTorch install path even though that path is not available in the source code anymore (hence the lint comment # type: ignore[import] # noqa: F401). We tested for situations where a PyTorch extension is built in an environment where PyTorch is already installed. Is that different from your use-case?

We use python setup.py develop for pytorch and torchvision

@shintaro-iwasaki @xwang233 I think the key here is that the way you are building torchvision, it is trying to pick up pytorch from the source code path (File "/opt/pytorch/pytorch/torch/utils/cpp_extension.py"), instead of the install path.

Hmm, does it mean that cd ~/git/pytorch/pytorch; python3 setup.py develop; cd ~/git/pytorch/vision; python3 setup.py develop would no longer works?

@shintaro-iwasaki Thank you for the suggestion. However, that won't work when the code tries to call hipify_python.hipify:

Traceback (most recent call last):
  File "setup.py", line 523, in <module>
    ext_modules=get_extensions(),
  File "setup.py", line 221, in get_extensions
    extra_compile_args=extra_compile_args,
  File "/var/lib/jenkins/pytorch/torch/utils/cpp_extension.py", line 1052, in CUDAExtension
    hipify_result = hipify_python.hipify(
AttributeError: 'NoneType' object has no attribute 'hipify'

Hmm, does it mean that cd ~/git/pytorch/pytorch; python3 setup.py develop; cd ~/git/pytorch/vision; python3 setup.py develop would no longer works?

@malfet Yes. We are working on an alternate PR so that the develop path would also work. Please review it here: fbc724b
cc @xwang233