Add conditional platform BLAS linking for Rust backend#167
Merged
Conversation
Link Apple Accelerate on macOS and OpenBLAS on Linux via Cargo features for matrix-vector and matrix-matrix products across all Rust-accelerated code paths (weights, OLS, TROP). Windows continues using pure Rust. - Add accelerate/openblas Cargo features with blas-src 0.10 dependency - Convert manual GEMV loop to ndarray general_mat_vec_mul in weights.rs (dispatches to BLAS dgemv when enabled, matrixmultiply fallback otherwise) - Add rust_backend_info() diagnostic function for BLAS feature reporting - Update CI workflows for platform-specific BLAS features - Add extern crate blas_src for linker flag propagation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Owner
Author
|
/ai-review |
The openblas-src -> openblas-build -> ureq -> native-tls dependency chain fails to compile on Ubuntu CI because native-tls 0.2.17 is incompatible with the latest Rust stable (missing Protocol::Tlsv13 match arm). Replace blas-src/openblas with a build.rs that emits `cargo:rustc-link-lib=openblas` directly, linking the system library (libopenblas-dev) without any download infrastructure. The accelerate feature (macOS) continues using blas-src as before. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
🔁 AI review rerun (requested by @igerber) Head SHA: Overall assessment: ✅ Looks good Executive Summary
Methodology Code Quality
Performance Maintainability Tech Debt Security Documentation/Tests Tests not run (not requested). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
sc_weight_fw_standardtondarray::linalg::general_mat_vec_mul(dispatches to BLASdgemvwhen enabled, falls back to ndarray'smatrixmultiplykernel)rust_backend_info()diagnostic function reporting compile-time BLAS feature statuspublish.yml,rust-test.yml) with platform-specific--featuresflags andopenblas-devel/libopenblas-devinstall stepsextern crate blas_srcfor linker flag propagation (required for linker-only crates)Methodology references (required if estimator / math changes)
Validation
tests/test_rust_backend.py: Addedtest_rust_backend_infotestrust/src/weights.rs: Addedtest_gemv_produces_correct_half_gradRust unit testmaturin develop --release --features accelerateon macOS ARM64rust_backend_info()correctly reports{'blas': True, 'accelerate': True, 'openblas': False}on macOSSecurity / privacy
Generated with Claude Code