This is an automated email from the ASF dual-hosted git repository. guanmingchiu pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/mahout.git
commit 4d3fc4b314196b9af9e9a09a496cfd1a3d2b8d9a Author: Ryan Huang <[email protected]> AuthorDate: Fri Jan 2 18:33:32 2026 +0800 [QDP] Pre commit rust followup (#766) * feat: use rust hook for precommit * Refactor clippy hook in .pre-commit-config.yaml Updated clippy hook configuration in pre-commit. * run linter --- .pre-commit-config.yaml | 24 +++++++++++++----------- qdp/qdp-core/src/gpu/cuda_ffi.rs | 2 -- qdp/qdp-core/src/gpu/pipeline.rs | 20 ++++++++++++++++++++ qdp/qdp-core/src/lib.rs | 10 ++++++---- qdp/qdp-kernels/tests/amplitude_encode.rs | 4 ++-- 5 files changed, 41 insertions(+), 19 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index a3a888049..a1058d495 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -59,17 +59,19 @@ repos: - "//" # Rust Linter - - repo: local + - repo: https://github.com/doublify/pre-commit-rust + rev: v1.0 hooks: - - id: rust-fmt - name: rustfmt - entry: bash -c 'cd qdp && cargo fmt --all' - language: system - types: [rust] + - id: fmt pass_filenames: false - - id: rust-clippy - name: clippy - entry: cargo clippy --manifest-path qdp/Cargo.toml --all-targets --all-features --fix --allow-dirty --allow-staged -- -D warnings - language: system - types: [rust] + args: ['--manifest-path', 'qdp/Cargo.toml', '--all'] + - id: clippy + # clippy needs context of the whole crate to compile correctly pass_filenames: false + args: [ + '--manifest-path', 'qdp/Cargo.toml', + '--all-targets', + '--all-features', + '--', + '-D', 'warnings' + ] diff --git a/qdp/qdp-core/src/gpu/cuda_ffi.rs b/qdp/qdp-core/src/gpu/cuda_ffi.rs index b61b4e4b2..fc4582a14 100644 --- a/qdp/qdp-core/src/gpu/cuda_ffi.rs +++ b/qdp/qdp-core/src/gpu/cuda_ffi.rs @@ -16,8 +16,6 @@ //! Centralized CUDA Runtime API FFI declarations. -#![cfg(target_os = "linux")] - use std::ffi::c_void; pub(crate) const CUDA_MEMCPY_HOST_TO_DEVICE: u32 = 1; diff --git a/qdp/qdp-core/src/gpu/pipeline.rs b/qdp/qdp-core/src/gpu/pipeline.rs index 7d206d1db..8c72bf3fa 100644 --- a/qdp/qdp-core/src/gpu/pipeline.rs +++ b/qdp/qdp-core/src/gpu/pipeline.rs @@ -70,6 +70,14 @@ impl PipelineContext { } /// Async H2D copy on copy stream + /// + /// # Safety + /// + /// The caller must ensure that: + /// - `dst` points to valid device memory of at least `len_elements * sizeof(f64)` bytes + /// - `src` is a valid pinned buffer with at least `len_elements` elements + /// - The memory regions do not overlap in an undefined way + /// - The CUDA stream is valid and properly initialized pub unsafe fn async_copy_to_device( &self, src: &PinnedBuffer, @@ -89,6 +97,10 @@ impl PipelineContext { } /// Record copy completion event + /// + /// # Safety + /// + /// The caller must ensure that the CUDA event and stream are valid and properly initialized. pub unsafe fn record_copy_done(&self) { unsafe { cudaEventRecord(self.event_copy_done, self.stream_copy.stream as *mut c_void); @@ -96,6 +108,10 @@ impl PipelineContext { } /// Make compute stream wait for copy completion + /// + /// # Safety + /// + /// The caller must ensure that the compute stream and copy event are valid and properly initialized. pub unsafe fn wait_for_copy(&self) { crate::profile_scope!("GPU::StreamWait"); unsafe { @@ -108,6 +124,10 @@ impl PipelineContext { } /// Sync copy stream (safe to reuse host buffer) + /// + /// # Safety + /// + /// The caller must ensure that the copy stream is valid and properly initialized. pub unsafe fn sync_copy_stream(&self) { crate::profile_scope!("Pipeline::SyncCopy"); unsafe { diff --git a/qdp/qdp-core/src/lib.rs b/qdp/qdp-core/src/lib.rs index d3301cbff..b8ff42fc6 100644 --- a/qdp/qdp-core/src/lib.rs +++ b/qdp/qdp-core/src/lib.rs @@ -35,6 +35,11 @@ use std::sync::mpsc::{Receiver, SyncSender, sync_channel}; #[cfg(target_os = "linux")] use std::thread; +#[cfg(target_os = "linux")] +type BufferResult = std::result::Result<(PinnedBuffer, usize), MahoutError>; +#[cfg(target_os = "linux")] +type BufferChannels = (SyncSender<BufferResult>, Receiver<BufferResult>); + use crate::dlpack::DLManagedTensor; #[cfg(target_os = "linux")] use crate::gpu::PipelineContext; @@ -200,10 +205,7 @@ impl QdpEngine { let dev_in_b = unsafe { self.device.alloc::<f64>(STAGE_SIZE_ELEMENTS) } .map_err(|e| MahoutError::MemoryAllocation(format!("{:?}", e)))?; - let (full_buf_tx, full_buf_rx): ( - SyncSender<std::result::Result<(PinnedBuffer, usize), MahoutError>>, - Receiver<std::result::Result<(PinnedBuffer, usize), MahoutError>>, - ) = sync_channel(2); + let (full_buf_tx, full_buf_rx): BufferChannels = sync_channel(2); let (empty_buf_tx, empty_buf_rx): (SyncSender<PinnedBuffer>, Receiver<PinnedBuffer>) = sync_channel(2); diff --git a/qdp/qdp-kernels/tests/amplitude_encode.rs b/qdp/qdp-kernels/tests/amplitude_encode.rs index beebbf068..f86e00fcb 100644 --- a/qdp/qdp-kernels/tests/amplitude_encode.rs +++ b/qdp/qdp-kernels/tests/amplitude_encode.rs @@ -509,9 +509,9 @@ fn test_amplitude_encode_small_input_large_state() { assert!((state_h[1].x - 0.8).abs() < EPSILON); // Rest should be zero - for i in 2..state_len { + for (i, item) in state_h.iter().enumerate().take(state_len).skip(2) { assert!( - state_h[i].x.abs() < EPSILON && state_h[i].y.abs() < EPSILON, + item.x.abs() < EPSILON && item.y.abs() < EPSILON, "Element {} should be zero-padded", i );
