cublas team verified workaround for large GEMM algo68 bug - #3100
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe cuBLASLt 13.6 workaround now uses version and leading-dimension checks, conditionally adjusts heuristic ChangescuBLASLt workaround
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cpp/include/raft/linalg/detail/cublaslt_wrappers.hpp`:
- Around line 229-254: Make the cublasLt matmul preference handle in the
heuristic setup exception-safe: replace the raw preference lifecycle around
query_heuristic with an existing RAII wrapper or add a scoped cleanup guard that
destroys it on every path, including when cublasLtMatmulAlgoGetHeuristic throws.
Preserve the existing heuristic queries and ensure destruction occurs exactly
once without relying on the trailing manual destroy call.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: e057ad64-8188-4d8f-b841-0068397d0a00
📒 Files selected for processing (2)
cpp/include/raft/linalg/detail/cublaslt_wrappers.hppcpp/tests/linalg/gemm_basic.cpp
Picked up real upstream rapidsai/raft changes between 26.08.00 and 26.10.00 (VERSION bumped as a side effect of the last origin/tuned-builds merge): native row-major PCA (NVIDIA#3036), predictable raft::resources (NVIDIA#3052), an nvtx/host-mem-resource symbol export fix (NVIDIA#3083), orphaned select_k declaration removal (NVIDIA#3084), move-safe cuBLASLt descriptor wrappers (NVIDIA#3078), cuda::std adoption in LAP kernels and stats minmax (NVIDIA#3094, NVIDIA#3095), and two real cuBLASLt/GEMM correctness fixes for CUDA 13.6 (NVIDIA#3098, NVIDIA#3100). No formal changelog exists yet for 26.08/26.10 (CHANGELOG.md's newest entry is still 26.06.00) -- compiled from git log between the two version points instead. Rebuilt, repackaged, and republished as v26.10-gb10-cu133 (https://github.com/zbrad/raft/releases/tag/v26.10-gb10-cu133), superseding v26.8-gb10-cu133 (deleted). Assisted-by: Claude Sonnet 5
The fix in #3098 was incomplete, as according to the cublas team there is no guarantee that
algo13will be returned along withalgo68(the failing version for A matrix with > 2^31 elements). Also, the cublas team will patch this bug in version13.6.1so we will automatically receive the fix by guarding only against13.6.0.