[Issue] `torch.version.hip` changed from `7.2.x` to `7.13.x` breaks ABI detection

[Apophis3158]

Projects such as ComfyUI, bitsandbytes use torch.version.hip to judge if ROCm ABI version 7.2.x or else. But lately TheRock build bumped it to 7.13.x in torch/version.py:

hip: Optional[str] = '7.13.x'
rocm: Optional[str] = '7.14.0'

IMO it's better to leave torch.version.hip to indicate the ABI version and only use torch.version.rocm to indicate TheRock ROCm version.

[Apophis3158]

This really matters.

For bitsandbytes:
https://github.com/bitsandbytes-foundation/bitsandbytes/blob/2177945befb0cf7ec5cfc726649579a361ca6b57/bitsandbytes/cuda_specs.py#L33-L34

bitsandbytes library load error: Configured ROCm binary not found at H:\ROCm\.venv\Lib\site-packages\bitsandbytes\libbitsandbytes_rocm713.dll
Traceback (most recent call last):
File "H:\ROCm\.venv\Lib\site-packages\bitsandbytes\cextension.py", line 366, in
lib = get_native_library()
File "H:\ROCm\.venv\Lib\site-packages\bitsandbytes\cextension.py", line 332, in get_native_library
raise RuntimeError(f"Configured {BNB_BACKEND} binary not found at {cuda_binary_path}")
RuntimeError: Configured ROCm binary not found at H:\ROCm\.venv\Lib\site-packages\bitsandbytes\libbitsandbytes_rocm713.dll

For ComfyUI:
https://github.com/Comfy-Org/ComfyUI/blob/cd45f42a83c75403e3b69f85789a2d3af43ced66/comfy/model_management.py#L487-L516

[lucbruni-amd]

Thanks for the report and the concrete examples @Apophis3158.

The HIP minor version bump from 7.2 to 7.13 in rocm-systems/projects/hip/VERSION is intentional at least according to history:

  ┌────────────┬─────────────────────────────────┬────────────────┐
  │   Commit   │         Commit message          │   HIP minor    │
  ├────────────┼─────────────────────────────────┼────────────────┤
  │ f27b8786b0 │ Bump minor version for ROCm 6.3 │ 2->3            │
  ├────────────┼─────────────────────────────────┼────────────────┤
  │ 862e713a5c │ Bump minor version for ROCm 6.4 │ 3->4            │
  ├────────────┼─────────────────────────────────┼────────────────┤
  │ 34ff076505 │ Bump minor version for ROCm 6.5 │ 4->5            │
  ├────────────┼─────────────────────────────────┼────────────────┤
  │ 0bd2233a69 │ Bump major version for ROCm 7.0 │ 6->7, minor 5->0 │
  ├────────────┼─────────────────────────────────┼────────────────┤
  │ 5b5f69c310 │ Bump minor version for ROCm 7.1 │ 0->1            │
  ├────────────┼─────────────────────────────────┼────────────────┤
  │ 9cac2e46e4 │ Bump minor version for ROCm 7.2 │ 1->2            │
  ├────────────┼─────────────────────────────────┼────────────────┤
  │ 3b1605a814 │ HIP Version Fixes (ROCm 7.13)   │ 2->13           │
  └────────────┴─────────────────────────────────┴────────────────┘

Both of these projects seem to be using torch.version.hip to derive a ROCm version identifier, looking at the snippets. I believe these downstream projects should be using torch.version.rocm since torch.version.hip can (and will likely) diverge in the future.

For bitsandbytes, there's an override mechanism as of bitsandbytes-foundation/bitsandbytes#1878 via the BNB_ROCM_VERSION environment variable, and an open PR bitsandbytes-foundation/bitsandbytes#1888 to use torch.version.rocm instead altogether. ComfyUI could benefit from a similar change.

I do think we can make address this better in our docs.

[Apophis3158]

It seems like commit ROCm/rocm-systems@3b1605a is only for fixing building issues rather than for bumping the torch.version.hip, this really broke things.

And switch to torch.version.rocm still get 7.14.x, not the ABI version 7.2.x we want.

[lucbruni-amd]

A couple clarifications:

• ROCm/rocm-systems@3b1605a did intentionally bump the HIP minor in addition to fixing build issues. TheRock builds were previously falling back to 7.2. Either way, these are all the same as far as ABI compatibility goes since the major version is the same, where the major version number is the compatibility boundary.

• Downstream projects should match on major version + highest-available <= minor version. Our bumps of the minor version when new API is added is not new and if we don't bump this version, we'll have other upstream components breaking.

I don't think there's anything we can do here in this case. You could attempt a PR but I don't believe the result will change.