fix(cmake): make cuTENSOR/cuQuantum finders honest and version-aware#993
fix(cmake): make cuTENSOR/cuQuantum finders honest and version-aware#993pcchen wants to merge 2 commits into
Conversation
Split out of #950 (finder-correctness portion). - CUTENSOR_FOUND / CUQUANTUM_FOUND are now derived from the actual find_library results via find_package_handle_standard_args, instead of being set TRUE unconditionally (which let a NOTFOUND silently pass a caller's REQUIRED check and link an empty / -NOTFOUND entry). - FindCUTENSOR: search both the cuTENSOR 2.x flat lib/ layout and the legacy per-CUDA-major lib/<major> layout (PATH_SUFFIXES), dropping the dead lib/10.2 and lib/11 branches; report the real found dir. - FindCUTENSOR: read CUTENSOR_MAJOR/MINOR from the headers and FATAL_ERROR on the unsupported 1.x API (Cytnx requires cuTENSOR >= 2.0). - FindCUQUANTUM: build CUQUANTUM_LIBRARIES conditionally (parity). Addresses the finder portions of #945 / #946. No CMakeLists.txt or compiler/version-floor policy changes (those remain separate splits). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the CMake module files FindCUQUANTUM.cmake and FindCUTENSOR.cmake to enforce stricter version requirements (CUDA >= 12 and cuTENSOR >= 2.0) and to use FindPackageHandleStandardArgs for robust package finding. Feedback on these changes suggests handling cases where the cuTENSOR minor version cannot be parsed by defaulting it to 0 to avoid malformed version strings, and dynamically retrieving CUQUANTUM_LIBRARY_DIRS for consistency.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| file(STRINGS "${_cutensor_version_header}" _cutensor_minor_line REGEX "^#define[ \t]+CUTENSOR_MINOR[ \t]+[0-9]+") | ||
| string(REGEX REPLACE ".*CUTENSOR_MAJOR[ \t]+([0-9]+).*" "\\1" CUTENSOR_VERSION_MAJOR "${_cutensor_major_line}") | ||
| string(REGEX REPLACE ".*CUTENSOR_MINOR[ \t]+([0-9]+).*" "\\1" CUTENSOR_VERSION_MINOR "${_cutensor_minor_line}") | ||
| set(CUTENSOR_VERSION "${CUTENSOR_VERSION_MAJOR}.${CUTENSOR_VERSION_MINOR}") |
There was a problem hiding this comment.
If CUTENSOR_MINOR is not found or fails to match the regex, _cutensor_minor_line will be empty. Performing string(REGEX REPLACE) on an empty string will result in CUTENSOR_VERSION_MINOR being empty, which leads to a malformed CUTENSOR_VERSION (e.g., 2.). This can cause version comparison failures in find_package_handle_standard_args or downstream scripts.
We should check if _cutensor_minor_line is non-empty before replacing, and default it to 0 otherwise.
file(STRINGS "${_cutensor_version_header}" _cutensor_minor_line REGEX "^#define[ \t]+CUTENSOR_MINOR[ \t]+[0-9]+")
string(REGEX REPLACE ".*CUTENSOR_MAJOR[ \t]+([0-9]+).*" "\\1" CUTENSOR_VERSION_MAJOR "${_cutensor_major_line}")
if(_cutensor_minor_line)
string(REGEX REPLACE ".*CUTENSOR_MINOR[ \t]+([0-9]+).*" "\\1" CUTENSOR_VERSION_MINOR "${_cutensor_minor_line}")
else()
set(CUTENSOR_VERSION_MINOR "0")
endif()
set(CUTENSOR_VERSION "${CUTENSOR_VERSION_MAJOR}.${CUTENSOR_VERSION_MINOR}")
| set(CUQUANTUM_LIBRARIES "") | ||
| if(CUQUANTUM_TENSORNET_LIB) | ||
| list(APPEND CUQUANTUM_LIBRARIES "${CUQUANTUM_TENSORNET_LIB}") | ||
| endif() | ||
| if(CUQUANTUM_CUSTATEVEC_LIB) | ||
| list(APPEND CUQUANTUM_LIBRARIES "${CUQUANTUM_CUSTATEVEC_LIB}") | ||
| endif() |
There was a problem hiding this comment.
For consistency with FindCUTENSOR.cmake and to ensure that CUQUANTUM_LIBRARY_DIRS accurately reflects the actual location of the found libraries (rather than assuming a hardcoded lib subdirectory), we should dynamically retrieve the directory of the found library using get_filename_component.
if(CUQUANTUM_TENSORNET_LIB)
get_filename_component(CUQUANTUM_LIBRARY_DIRS "${CUQUANTUM_TENSORNET_LIB}" DIRECTORY)
endif()
set(CUQUANTUM_LIBRARIES "")
if(CUQUANTUM_TENSORNET_LIB)
list(APPEND CUQUANTUM_LIBRARIES "${CUQUANTUM_TENSORNET_LIB}")
endif()
if(CUQUANTUM_CUSTATEVEC_LIB)
list(APPEND CUQUANTUM_LIBRARIES "${CUQUANTUM_CUSTATEVEC_LIB}")
endif()
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0c19d7e4c6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #993 +/- ##
=======================================
Coverage 30.88% 30.88%
=======================================
Files 229 229
Lines 34758 34758
Branches 14409 14409
=======================================
Hits 10734 10734
Misses 16720 16720
Partials 7304 7304
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
…arse - FindCUTENSOR: drop the CUDA>=12 FATAL_ERROR (a version-floor policy that belongs in the separate #962/CMakeLists.txt split and would reject valid CUDA 11 + cuTENSOR 2.x installs). The generic lib/<major> suffix already covers 11/12/13. This finder now stays policy-free. (codex review) - FindCUTENSOR: default CUTENSOR_MINOR to 0 when the header line is absent, so CUTENSOR_VERSION isn't malformed ("2.") for the VERSION_VAR check. (gemini review) - FindCUQUANTUM: derive CUQUANTUM_LIBRARY_DIRS from the found library via get_filename_component, mirroring FindCUTENSOR. (gemini review) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Thanks — addressed all three in
Note this PR (first split of #950) intentionally keeps only finder correctness + layout + the cuTENSOR ≥ 2.0 API requirement — no CMakeLists.txt / CUDA-version-floor changes. As before, it can't be built on a CUDA-less machine; a reviewer with a CUDA box please sanity-check a Posted by Claude Code on behalf of @pcchen |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6650adcbfd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| message(WARNING | ||
| "Could not determine the cuTENSOR version from headers under " | ||
| "${CUTENSOR_INCLUDE_DIRS}; Cytnx requires cuTENSOR >= 2.0.") |
There was a problem hiding this comment.
When CUTENSOR_ROOT has a libcutensor library but the headers are missing or don't expose CUTENSOR_MAJOR in one of these two searched files, this branch only emits a warning. The subsequent find_package_handle_standard_args(... VERSION_VAR CUTENSOR_VERSION) does not fail because the repo calls find_package(CUTENSOR REQUIRED) without requesting a version, so CUTENSOR_FOUND becomes true with an empty/unknown version and UNI_CUTENSOR is enabled anyway. That defers an invalid cuTENSOR install to compile/link time, and can also let unsupported headers through when the new >=2 check cannot parse them.
Useful? React with 👍 / 👎.
| # Search both cuTENSOR library layouts: 2.x tarballs place the libraries | ||
| # directly under lib/, while older tarballs and apt use a per-CUDA-major | ||
| # subdir (lib/<cuda-major>, e.g. lib/11, lib/12, lib/13). Listing both as |
There was a problem hiding this comment.
This layout was changed since 2.3 not 2.x
| # directly under lib/, while older tarballs and apt use a per-CUDA-major | ||
| # subdir (lib/<cuda-major>, e.g. lib/11, lib/12, lib/13). Listing both as | ||
| # find_library PATH_SUFFIXES resolves either layout for any CUDA major. The | ||
| # older minor-specific lib/10.2 and lib/11.0 special-cases were removed; the |
There was a problem hiding this comment.
This comment is not useful for the future. It can be put in the commit message.
| # generic lib/<major> covers them (apt multiarch paths remain, issue #946). | ||
| # The CUDA-version floor is a separate policy decision (issue #962), enforced | ||
| # in CMakeLists.txt rather than gated here, so this finder stays policy-free. | ||
| set(CUTNLIB_DIR lib lib/${CUDAToolkit_VERSION_MAJOR}) |
There was a problem hiding this comment.
CUTNLIB_DIR is not used in the new implementation.
2 participants
First split of #950 (the finder-correctness portion). No CMakeLists.txt or
compiler/version-floor changes — those remain separate follow-up splits.
Changes
*_FOUND.CUTENSOR_FOUND/CUQUANTUM_FOUNDare now derived fromthe actual
find_libraryresults viafind_package_handle_standard_args,instead of
set(... TRUE)unconditionally — which let aNOTFOUNDsilentlypass a caller's
REQUIREDcheck and link an empty /-NOTFOUNDentry.lib/and the legacy per-CUDA-major
lib/<major>viaPATH_SUFFIXES, dropping thedead
lib/10.2/lib/11branches; report the directory actually found.CUTENSOR_MAJOR/MINORfrom the headersand failing early on the 1.x API.
CUQUANTUM_LIBRARIESconditionally (parity).Addresses the finder portions of #945 / #946.
Verification
Extracted verbatim from the already-reviewed #950 diff; both modules parse
cleanly. These finders only run under
USE_CUTENSOR/USE_CUQUANTUM(i.e. aCUDA build), which can't be exercised on the author's macOS machine — a
reviewer with a CUDA box please sanity-check a
-DUSE_CUTENSOR=ON(and
-DUSE_CUQUANTUM=ON) configure.Posted by Claude Code on behalf of @pcchen