Re: [PR] [QDP] Refactor `encode()` method into helper functions with tests [mahout]
viiccwen commented on PR #814: URL: https://github.com/apache/mahout/pull/814#issuecomment-3791049344 Thanks for reviewing! 🙌 -- 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]
Re: [PR] [QDP] Refactor `encode()` method into helper functions with tests [mahout]
guan404ming merged PR #814: URL: https://github.com/apache/mahout/pull/814 -- 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]
Re: [PR] [QDP] Refactor `encode()` method into helper functions with tests [mahout]
viiccwen commented on PR #814: URL: https://github.com/apache/mahout/pull/814#issuecomment-3789366909 another conflict 💀 -- 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]
Re: [PR] [QDP] Refactor `encode()` method into helper functions with tests [mahout]
rich7420 commented on code in PR #814:
URL: https://github.com/apache/mahout/pull/814#discussion_r2719951452
##
qdp/qdp-python/src/lib.rs:
##
@@ -238,37 +238,103 @@ fn validate_cuda_tensor_for_encoding(
Ok(())
}
-/// CUDA tensor information extracted directly from PyTorch tensor
-struct CudaTensorInfo {
+/// DLPack tensor information extracted from a PyCapsule
+///
+/// This struct owns the DLManagedTensor pointer and ensures proper cleanup
+/// via the DLPack deleter when dropped (RAII pattern).
+struct DLPackTensorInfo {
+/// Raw DLManagedTensor pointer from PyTorch DLPack capsule
+/// This is owned by this struct and will be freed via deleter on drop
+managed_ptr: *mut DLManagedTensor,
+/// Data pointer inside dl_tensor (GPU memory, owned by managed_ptr)
data_ptr: *const f64,
shape: Vec,
+/// CUDA device ID from DLPack metadata.
+/// Currently unused but kept for potential future device validation or
multi-GPU support.
+#[allow(dead_code)]
+device_id: i32,
Review Comment:
no problem, plz open a issue.
--
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]
Re: [PR] [QDP] Refactor `encode()` method into helper functions with tests [mahout]
viiccwen commented on code in PR #814:
URL: https://github.com/apache/mahout/pull/814#discussion_r2719912974
##
qdp/qdp-python/src/lib.rs:
##
@@ -358,237 +424,264 @@ impl QdpEngine {
// Check if it's a NumPy array
if data.hasattr("__array_interface__")? {
-// Get the array's ndim for shape validation
-let ndim: usize = data.getattr("ndim")?.extract()?;
-
-match ndim {
-1 => {
-// 1D array: single sample encoding (zero-copy if already
contiguous)
-let array_1d =
data.extract::>().map_err(|_| {
-PyRuntimeError::new_err(
-"Failed to extract 1D NumPy array. Ensure dtype is
float64.",
-)
-})?;
-let data_slice = array_1d.as_slice().map_err(|_| {
-PyRuntimeError::new_err("NumPy array must be
contiguous (C-order)")
-})?;
-let ptr = self
-.engine
-.encode(data_slice, num_qubits, encoding_method)
-.map_err(|e| PyRuntimeError::new_err(format!("Encoding
failed: {}", e)))?;
-return Ok(QuantumTensor {
-ptr,
-consumed: false,
-});
-}
-2 => {
-// 2D array: batch encoding (zero-copy if already
contiguous)
-let array_2d =
data.extract::>().map_err(|_| {
-PyRuntimeError::new_err(
-"Failed to extract 2D NumPy array. Ensure dtype is
float64.",
-)
-})?;
-let shape = array_2d.shape();
-let num_samples = shape[0];
-let sample_size = shape[1];
-let data_slice = array_2d.as_slice().map_err(|_| {
-PyRuntimeError::new_err("NumPy array must be
contiguous (C-order)")
-})?;
-let ptr = self
-.engine
-.encode_batch(
-data_slice,
-num_samples,
-sample_size,
-num_qubits,
-encoding_method,
-)
-.map_err(|e| PyRuntimeError::new_err(format!("Encoding
failed: {}", e)))?;
-return Ok(QuantumTensor {
-ptr,
-consumed: false,
-});
-}
-_ => {
-return Err(PyRuntimeError::new_err(format!(
-"Unsupported array shape: {}D. Expected 1D array for
single sample \
- encoding or 2D array (batch_size, features) for batch
encoding.",
-ndim
-)));
-}
-}
+return self.encode_from_numpy(data, num_qubits, encoding_method);
}
// Check if it's a PyTorch tensor
if is_pytorch_tensor(data)? {
-// Check if it's a CUDA tensor - use zero-copy GPU encoding
-if is_cuda_tensor(data)? {
-// Validate CUDA tensor for direct GPU encoding
-validate_cuda_tensor_for_encoding(
-data,
-self.engine.device().ordinal(),
-encoding_method,
-)?;
-
-// Extract GPU pointer directly from PyTorch tensor
-let tensor_info = extract_cuda_tensor_info(data)?;
-
-let ndim: usize = data.call_method0("dim")?.extract()?;
-
-match ndim {
-1 => {
-// 1D CUDA tensor: single sample encoding
-let input_len = tensor_info.shape[0] as usize;
-// SAFETY: tensor_info.data_ptr was obtained via
PyTorch's data_ptr() from a
-// valid CUDA tensor. The tensor remains alive during
this call
-// (held by Python's GIL), and we validated
dtype/contiguity/device above.
-let ptr = unsafe {
-self.engine
-.encode_from_gpu_ptr(
-tensor_info.data_ptr,
-input_len,
-num_qubits,
-encoding_method,
-)
-.map_err(|e| {
-PyRuntimeError::new_err(format!("Encoding
failed: {}", e))
-})?
-};
-return Ok(Q
Re: [PR] [QDP] Refactor `encode()` method into helper functions with tests [mahout]
viiccwen commented on code in PR #814:
URL: https://github.com/apache/mahout/pull/814#discussion_r2719910017
##
qdp/qdp-python/src/lib.rs:
##
@@ -238,37 +238,103 @@ fn validate_cuda_tensor_for_encoding(
Ok(())
}
-/// CUDA tensor information extracted directly from PyTorch tensor
-struct CudaTensorInfo {
+/// DLPack tensor information extracted from a PyCapsule
+///
+/// This struct owns the DLManagedTensor pointer and ensures proper cleanup
+/// via the DLPack deleter when dropped (RAII pattern).
+struct DLPackTensorInfo {
+/// Raw DLManagedTensor pointer from PyTorch DLPack capsule
+/// This is owned by this struct and will be freed via deleter on drop
+managed_ptr: *mut DLManagedTensor,
+/// Data pointer inside dl_tensor (GPU memory, owned by managed_ptr)
data_ptr: *const f64,
shape: Vec,
+/// CUDA device ID from DLPack metadata.
+/// Currently unused but kept for potential future device validation or
multi-GPU support.
+#[allow(dead_code)]
+device_id: i32,
Review Comment:
IMO, it could be a follow-up. WDYT?
bcz this PR only deals with refactoring encode path & adding missing tests.
--
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]
Re: [PR] [QDP] Refactor `encode()` method into helper functions with tests [mahout]
rich7420 commented on code in PR #814:
URL: https://github.com/apache/mahout/pull/814#discussion_r2719829745
##
qdp/qdp-python/src/lib.rs:
##
@@ -358,237 +424,264 @@ impl QdpEngine {
// Check if it's a NumPy array
if data.hasattr("__array_interface__")? {
-// Get the array's ndim for shape validation
-let ndim: usize = data.getattr("ndim")?.extract()?;
-
-match ndim {
-1 => {
-// 1D array: single sample encoding (zero-copy if already
contiguous)
-let array_1d =
data.extract::>().map_err(|_| {
-PyRuntimeError::new_err(
-"Failed to extract 1D NumPy array. Ensure dtype is
float64.",
-)
-})?;
-let data_slice = array_1d.as_slice().map_err(|_| {
-PyRuntimeError::new_err("NumPy array must be
contiguous (C-order)")
-})?;
-let ptr = self
-.engine
-.encode(data_slice, num_qubits, encoding_method)
-.map_err(|e| PyRuntimeError::new_err(format!("Encoding
failed: {}", e)))?;
-return Ok(QuantumTensor {
-ptr,
-consumed: false,
-});
-}
-2 => {
-// 2D array: batch encoding (zero-copy if already
contiguous)
-let array_2d =
data.extract::>().map_err(|_| {
-PyRuntimeError::new_err(
-"Failed to extract 2D NumPy array. Ensure dtype is
float64.",
-)
-})?;
-let shape = array_2d.shape();
-let num_samples = shape[0];
-let sample_size = shape[1];
-let data_slice = array_2d.as_slice().map_err(|_| {
-PyRuntimeError::new_err("NumPy array must be
contiguous (C-order)")
-})?;
-let ptr = self
-.engine
-.encode_batch(
-data_slice,
-num_samples,
-sample_size,
-num_qubits,
-encoding_method,
-)
-.map_err(|e| PyRuntimeError::new_err(format!("Encoding
failed: {}", e)))?;
-return Ok(QuantumTensor {
-ptr,
-consumed: false,
-});
-}
-_ => {
-return Err(PyRuntimeError::new_err(format!(
-"Unsupported array shape: {}D. Expected 1D array for
single sample \
- encoding or 2D array (batch_size, features) for batch
encoding.",
-ndim
-)));
-}
-}
+return self.encode_from_numpy(data, num_qubits, encoding_method);
}
// Check if it's a PyTorch tensor
if is_pytorch_tensor(data)? {
-// Check if it's a CUDA tensor - use zero-copy GPU encoding
-if is_cuda_tensor(data)? {
-// Validate CUDA tensor for direct GPU encoding
-validate_cuda_tensor_for_encoding(
-data,
-self.engine.device().ordinal(),
-encoding_method,
-)?;
-
-// Extract GPU pointer directly from PyTorch tensor
-let tensor_info = extract_cuda_tensor_info(data)?;
-
-let ndim: usize = data.call_method0("dim")?.extract()?;
-
-match ndim {
-1 => {
-// 1D CUDA tensor: single sample encoding
-let input_len = tensor_info.shape[0] as usize;
-// SAFETY: tensor_info.data_ptr was obtained via
PyTorch's data_ptr() from a
-// valid CUDA tensor. The tensor remains alive during
this call
-// (held by Python's GIL), and we validated
dtype/contiguity/device above.
-let ptr = unsafe {
-self.engine
-.encode_from_gpu_ptr(
-tensor_info.data_ptr,
-input_len,
-num_qubits,
-encoding_method,
-)
-.map_err(|e| {
-PyRuntimeError::new_err(format!("Encoding
failed: {}", e))
-})?
-};
-return Ok(Q
Re: [PR] [QDP] Refactor `encode()` method into helper functions with tests [mahout]
guan404ming commented on PR #814: URL: https://github.com/apache/mahout/pull/814#issuecomment-3773495356 Make sense, thanks. -- 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]
Re: [PR] [QDP] Refactor `encode()` method into helper functions with tests [mahout]
viiccwen commented on PR #814: URL: https://github.com/apache/mahout/pull/814#issuecomment-3773488192 replying here! https://github.com/apache/mahout/pull/881#issuecomment-3773452490 > I'll dive into refactor encode (qdp/qdp-python/src/lib.rs) after this PR merged. -- 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]
Re: [PR] [QDP] Refactor `encode()` method into helper functions with tests [mahout]
guan404ming commented on PR #814: URL: https://github.com/apache/mahout/pull/814#issuecomment-3773467738 Hey @viiccwen, how this going? Feel free let me know if you need any help. Thanks! -- 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]
Re: [PR] [QDP] Refactor `encode()` method into helper functions with tests [mahout]
viiccwen commented on PR #814: URL: https://github.com/apache/mahout/pull/814#issuecomment-3764643052 > please resolve the conflict and re-think the solution with newly introduced testing refactor if needed Sure, I'll kick off today or tomorrow! -- 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]
Re: [PR] [QDP] Refactor `encode()` method into helper functions with tests [mahout]
ryankert01 commented on PR #814: URL: https://github.com/apache/mahout/pull/814#issuecomment-3758414926 please resolve the conflict and re-think the solution with newly introduced testing refactor if needed -- 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]
Re: [PR] [QDP] Refactor `encode()` method into helper functions with tests [mahout]
rich7420 commented on PR #814: URL: https://github.com/apache/mahout/pull/814#issuecomment-3752761412 plz resolve the pre-commit error -- 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]
Re: [PR] [QDP] Refactor `encode()` method into helper functions with tests [mahout]
400Ping commented on PR #814: URL: https://github.com/apache/mahout/pull/814#issuecomment-3748315300 Please resolve conflicts -- 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]
Re: [PR] [QDP] Refactor `encode()` method into helper functions with tests [mahout]
viiccwen commented on PR #814: URL: https://github.com/apache/mahout/pull/814#issuecomment-3739504999 cc @400Ping, @guan404ming 🙌 -- 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]
