Skip to content

[ROCm] Fix round for #1399: merge upstream main, resolve conflicts - #1

Merged
jeffdaily merged 18 commits into
moat-portfrom
moat-fix-1399
Aug 17, 2026
Merged

[ROCm] Fix round for #1399: merge upstream main, resolve conflicts#1
jeffdaily merged 18 commits into
moat-portfrom
moat-fix-1399

Conversation

@jeffdaily

Copy link
Copy Markdown
Collaborator

Fix round on upstream PR apache#1399, staged on moat-fix-1399 (base fd8c7a38, tip 9a3a08e0). Answers ryankert01's CHANGES_REQUESTED ("Need to resolve conflicts" — the PR reads CONFLICTING against upstream main) and viiccwen's 2026-08-16 "any updates?" ping.

What changed

fd8c7a38 is merged (never rebased) with upstream/main — 16 upstream commits, 26 files in qdp/. Two commits of ours on top of upstream's:

  • 5a15c3f [ROCm] Merge upstream main into the AMD/HIP branch. Four conflicts, all at the seams between upstream's new work and the HIP path; each resolution keeps both sides. The one substantive addition: upstream's new cuda_runtime_available() helper (from [MAHOUT-1414][QDP] Expose native cuda_available() and gate GPU tests on it apache/mahout#1416) calls cudaGetDeviceCount, which needed the HIP-side wrapper (cudaGetDeviceCount -> hipGetDeviceCount) added in cuda_ffi.rs — without it the hip build does not compile. Post-merge diff fd8c7a38..tip -- qdp/ verified to be exactly upstream's 26-file delta plus that 6-line wrapper: no port intent lost.
  • 9a3a08e [ROCm] Gate new Parquet fidelity GPU tests on the GPU cfg. Upstream's new parquet_f32_fidelity.rs gated its GPU module on target_os = "linux" where every other GPU test uses qdp_gpu_platform; on a Windows ROCm build those three GPU cases would silently vanish. Test-only change.

(The reviewer's must-fix from 2026-08-13 — the merge commit lacked the [ROCm] prefix — was resolved by rewording while the branch was unpublished; the published tip remains parent 1 and an ancestor, so the PR fast-forward stays possible.)

What revalidated (all at tip 9a3a08e)

  • linux-gfx1100 (W7800): Rust suite 368 passed / 0 failed (baseline 353 at the published tip, +15 accounted exactly by upstream's 14 new tests + 1 new doctest — every new upstream test runs and passes on AMD, none skipped).
  • linux-gfx90a (MI250X): same suite, pass.
  • windows-gfx1151 (Radeon 8060S): Rust 368 passed, plus the Python parity recipe since the delta touches qdp-python directly — wheel rebuilt, _qdp.cuda_available() returns True on the HIP build, pytest 286 passed / 31 skipped / 0 failed (all skips explained; improvement over the prior baseline).
  • CUDA (default feature) compile check: real nvcc 12.8, build + --no-run test binaries, exit 0 (recorded at this head by linux-gfx1100; no .cu kernel source in the delta).
  • Staging tip contains current upstream main (merge-base --is-ancestor both directions verified; merge-tree clean), so the PR flips out of CONFLICTING on fast-forward.
  • Jargon over main..moat-fix-1399 (commits and added lines): clean. Fork tree clean.

Upstream reply

Conflicts resolved by merging current main into the branch (merge, not rebase, so the review history stands). Four conflicts, all at the seams between the new cuda_available()/max-qubit-validation work and the HIP path; each resolution keeps both sides. One addition was needed to compile: the new cuda_runtime_available() helper calls cudaGetDeviceCount, so the HIP build now maps that to hipGetDeviceCount alongside the other runtime entry points -- it correctly reports AMD devices too.

One small follow-up commit: the new Parquet f32 fidelity tests gated their GPU module on target_os = "linux", while the rest of the GPU tests use the build-script cfg that also covers Windows ROCm builds; switched it to match so those three cases run there as well.

Re-ran everything at the merged tip: Rust suite 368 passed / 0 failed on Linux (MI250X gfx90a, Radeon Pro W7800 gfx1100) and Windows (Radeon 8060S gfx1151); Python bindings rebuilt on Windows, 286 passed / 0 failed, with the new cuda_available() returning true on the HIP build; the default CUDA feature still compiles clean with nvcc 12.8. All of the new upstream tests (estimate, Parquet fidelity, the excessive-qubit case) run and pass on AMD hardware.

For our own record (not posted upstream)

Approving covers the merge, the follow-up commit, and the reply above; upstream.py --merge-fix --apply then fast-forwards apache#1399's branch to 9a3a08e0 and posts the reply, which also answers the stale-bot/viiccwen pings.

ryankert01 and others added 18 commits July 3, 2026 12:17
* f32 Parquet fidelity tests and benchmark

* fix error

* address comment
…on it (apache#1416)

Follow-up to apache#1321 (addresses apache#1414). After apache#1321, `_qdp` can build and
import without the CUDA toolkit (stub runtime), so "extension importable"
no longer implies a usable GPU. The test suite gated GPU tests on
`torch.cuda.is_available()`, a proxy that is wrong on this PR's headline
scenario -- a GPU host with PyTorch but no toolkit, where `_qdp` is a stub
build yet torch still reports a device.

- qdp-core: add `cuda_runtime_available()`, which queries
  `cudaGetDeviceCount` (false in a stub build via the existing 999 sentinel
  stub, and on hosts with no device). Re-exported from the crate root.
- _qdp: expose it as `_qdp.cuda_available()`.
- qumat_qdp: add `is_cuda_available()`, mirroring `is_triton_amd_available()`,
  as the single Python source of truth.
- testing/conftest: gate the `@pytest.mark.gpu` auto-skip on the native
  signal (falling back to torch only if the helper is absent).
- test_fallback: coverage that runs on a stub build too, guarding that
  querying availability returns a bool without aborting.

Verified on GPU (tests run and pass) and with CUDA hidden (tests skip);
full Rust suite, clippy --all-features, ruff, and ty all clean.
… angle and basis (apache#1439)

Signed-off-by: viiccwen <vicwen@apache.org>
Signed-off-by: viiccwen <vicwen@apache.org>
Bumps [pyasn1](https://github.com/pyasn1/pyasn1) from 0.6.3 to 0.6.4.
- [Release notes](https://github.com/pyasn1/pyasn1/releases)
- [Changelog](https://github.com/pyasn1/pyasn1/blob/main/CHANGES.rst)
- [Commits](pyasn1/pyasn1@v0.6.3...v0.6.4)

---
updated-dependencies:
- dependency-name: pyasn1
  dependency-version: 0.6.4
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…pache#1444)

Bumps [torch](https://github.com/pytorch/pytorch) from 2.12.0 to 2.13.0.
- [Release notes](https://github.com/pytorch/pytorch/releases)
- [Changelog](https://github.com/pytorch/pytorch/blob/main/RELEASE.md)
- [Commits](pytorch/pytorch@v2.12.0...v2.13.0)

---
updated-dependencies:
- dependency-name: torch
  dependency-version: 2.13.0
  dependency-type: direct:development
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [pillow](https://github.com/python-pillow/Pillow) from 12.2.0 to 12.3.0.
- [Release notes](https://github.com/python-pillow/Pillow/releases)
- [Changelog](https://github.com/python-pillow/Pillow/blob/main/CHANGES.rst)
- [Commits](python-pillow/Pillow@12.2.0...12.3.0)

---
updated-dependencies:
- dependency-name: pillow
  dependency-version: 12.3.0
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [setuptools](https://github.com/pypa/setuptools) from 80.9.0 to 83.0.0.
- [Release notes](https://github.com/pypa/setuptools/releases)
- [Changelog](https://github.com/pypa/setuptools/blob/main/NEWS.rst)
- [Commits](pypa/setuptools@v80.9.0...v83.0.0)

---
updated-dependencies:
- dependency-name: setuptools
  dependency-version: 83.0.0
  dependency-type: direct:development
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…#1455)

Bumps [quinn-proto](https://github.com/quinn-rs/quinn) from 0.11.14 to 0.11.16.
- [Release notes](https://github.com/quinn-rs/quinn/releases)
- [Commits](quinn-rs/quinn@quinn-proto-0.11.14...quinn-proto-0.11.16)

---
updated-dependencies:
- dependency-name: quinn-proto
  dependency-version: 0.11.16
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: viiccwen <vicwen@apache.org>
Signed-off-by: viiccwen <vicwen@apache.org>
Upstream moved on since this branch was last synchronized, leaving the pull
request conflicting. Merge current main and resolve the four conflicts so it
applies cleanly again. Nothing is rebased: the previously reviewed commits stay
exactly where they were.

qdp-core/src/gpu/cuda_ffi.rs: upstream added a cudaGetDeviceCount binding and the
cuda_runtime_available() helper that reports whether a usable device exists. Both
are kept, and the HIP backend gains the matching hipGetDeviceCount wrapper, so the
helper answers for AMD devices on the ROCm build instead of leaving an unresolved
name. The three-way backend split (real CUDA runtime, no-toolkit stubs, HIP) is
otherwise unchanged, and the stub backend already carried upstream's new entry
point.

qdp-core/src/gpu/encodings/amplitude.rs: kept upstream's shared
validate_qubit_count import next to the qdp_gpu_platform cfg this branch uses in
place of a bare target_os check, so the new qubit-limit validation applies on both
vendors.

qdp-core/tests/gpu_ptr_encoding.rs: kept upstream's new MAX_QUBITS imports for the
excessive-qubit test. The device-pointer traits keep coming from
qdp_core::gpu_rt, which resolves to cudarc on CUDA and to the HIP shim on ROCm.

qdp-python/src/lib.rs: registered upstream's new cuda_available() function and
kept the qdp_gpu_platform cfg on the loader and pipeline registrations.

Authored with assistance from Claude (Anthropic).

Test Plan:
Linux gfx1100 (Radeon Pro W7800, ROCm 7.2.1) for the HIP path, and a CUDA 12.8
compile check on the same host (no NVIDIA GPU present, so CUDA is build-only).

```
export QDP_USE_HIP=1 QDP_HIP_ARCH_LIST=gfx1100 ROCM_PATH=/opt/rocm
cargo build -p qdp-core -p qdp-kernels --no-default-features --features hip -j 16
HIP_VISIBLE_DEVICES=0 cargo test -p qdp-core -p qdp-kernels \
  --no-default-features --features hip -- --test-threads=1   # 368 passed, 0 failed
```

```
export CUDA_PATH=/opt/conda/envs/cuda-12.8 PATH=$CUDA_PATH/bin:$PATH
cargo build -p qdp-core -p qdp-kernels -j 16                 # nvcc 12.8, exit 0
cargo test -p qdp-core -p qdp-kernels --no-run -j 16         # 23 test binaries
```

The AMD run includes upstream's newly merged suites: estimate (9) and
parquet_f32_fidelity (4, including the three GPU fidelity cases), plus the new
excessive-qubit case in gpu_ptr_encoding (69, was 68) and the estimate_memory doc
example. That is 368 passing tests on this GPU, up from 353 before the merge.
The Parquet f32 fidelity suite that just merged gates its GPU module on
target_os = "linux". Every other GPU test in the crate uses the qdp_gpu_platform
cfg the build script emits, which is set on Linux and also on Windows when the
HIP feature is on, so this one file would silently drop its three GPU cases on a
Windows ROCm build while the rest of the suite runs. Use the same cfg here, and
drop the "Linux + CUDA" wording from the header comments since the module now
covers either vendor's GPU backend.

Test only; no library or kernel code changes.

Authored with assistance from Claude (Anthropic).

Test Plan:
Linux gfx1100 (Radeon Pro W7800, ROCm 7.2.1).

```
export QDP_USE_HIP=1 QDP_HIP_ARCH_LIST=gfx1100 ROCM_PATH=/opt/rocm
HIP_VISIBLE_DEVICES=0 cargo test -p qdp-core --no-default-features --features hip \
  --test parquet_f32_fidelity -- --test-threads=1
```

4 passed, 0 failed: the CPU reader-consistency case plus the three GPU fidelity
cases at 8, 12 and 16 qubits.
jeffdaily added a commit to AMD-Ecosystem/moat that referenced this pull request Aug 17, 2026
@jeffdaily

Copy link
Copy Markdown
Collaborator Author

/moat approve

@jeffdaily
jeffdaily merged commit 9a3a08e into moat-port Aug 17, 2026
@jeffdaily
jeffdaily deleted the moat-fix-1399 branch August 17, 2026 20:40
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.

4 participants