aloha1357 commented on PR #1389:
URL: https://github.com/apache/mahout/pull/1389#issuecomment-5074590751
Hi @rich7420 ,
Thank you for the detailed review! I've addressed all four of your
inline comments in the latest push.
Here is a summary of the fixes:
1. **`sm_75` Architecture Regression**:
I've split the `cc::Build` targets in `build.rs`. Legacy kernels
(e.g., `iqp.cu`, `amplitude.cu`) are
now correctly compiled with `sm_75` and above, restoring support for
Turing GPUs. The Tensor Core specific
kernels (`ImplicitHadamardOzaki.cu` and `AdaptiveOzaki.cu`) remain
strictly targeted at `sm_80` and above.
2. **Silent Skipping in Python Tests**:
I updated the Python fixture in `test_iqp_tc_path.py` and added a
rust-level `tc_smoke.rs` test. If
`QDP_REQUIRE_GPU=1` is set in the environment, the tests will now
explicitly fail instead of silently
skipping, making CI regressions highly visible.
3. **Missing `cudaMalloc` Error Checks**:
I added a `CHECK_ALLOC` macro in `AdaptiveOzaki.cu` to gracefully
catch and handle `cudaMalloc` failures
during workspace allocation, preventing writes to null pointers under low
VRAM conditions.
4. **Misleading `launch_adaptive_ozaki_gemm` API**:
Since it is currently unused from the Rust side and its stream
ordering guarantee was misleading (due to
internal hybrid streams), I've removed this entry point entirely for now
to keep the API surface clean.
All tests and pre-commit checks are green. Please let me know if there's
anything else you'd like me to
refine!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
