Add safe casting in Cython bindings - #8062
Conversation
📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughThis PR introduces comprehensive dimension/integer bounds validation across cuML's Python bindings to prevent overflow when passing Python integers to C/C++/CUDA native code. A new ChangesDimension Validation Infrastructure & Integration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
python/cuml/cuml/linear_model/base_mg.pyx (1)
77-78:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't force every sparse
X_nnzthrough the 32-bit path.
self.index_dtypeis already captured on the sparse path, but Line 122 always applies anintbound. That blocks valid 64-bit sparse inputs before model-specific_fitimplementations can take theirint64_toverloads (for exampleLogisticRegressionMG._fitin this PR). Gate this check on the index dtype, or use a 64-bit-safe bound for the 64-bit path instead.Also applies to: 121-122
🤖 Prompt for 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. In `@python/cuml/cuml/linear_model/base_mg.pyx` around lines 77 - 78, The current check forces sparse X_nnz through a 32-bit int bound unconditionally, blocking valid 64-bit sparse inputs; modify the logic around self.index_dtype (set from X_m.indptr.dtype) so that the 32-bit bound is only applied when self.index_dtype is a 32-bit integer type, and when self.index_dtype is 64-bit either skip the 32-bit cast or use a 64-bit-safe bound (int64_t) before calling the model-specific _fit (e.g., LogisticRegressionMG._fit) so 64-bit overloads can be reached; update the conditional around X_nnz handling to branch on self.index_dtype rather than always applying the 32-bit path.python/cuml/cuml/cluster/spectral_clustering.pyx (1)
315-315:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate Python values before assigning to
cdef intfields.The code assigns
affinity_nnz(line 329),config.n_clusters(line 350), andconfig.n_init(line 356) toint-backed storage before callingdims_within_int_limits(lines 358–361). Use Python temporaries first and validate them, matching the pattern used forn_samplesandn_featuresat lines 307–310.🤖 Prompt for 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. In `@python/cuml/cuml/cluster/spectral_clustering.pyx` at line 315, Assign Python temporaries for the values that will be stored in C ints (affinity_nnz, config.n_clusters, config.n_init) and validate them with dims_within_int_limits before writing into the cdef int fields; follow the same pattern used for n_samples and n_features (create py_* temporaries, call dims_within_int_limits on those, then assign to affinity_nnz, config.n_clusters, config.n_init) to avoid truncation or overflow when populating the C-backed fields in spectral_clustering.pyx.python/cuml/cuml/solvers/sgd.pyx (1)
111-121:⚠️ Potential issue | 🟠 Major | ⚡ Quick winThese bounds checks run after Cython has already coerced the hyperparameters to
int.
epochs,batch_size, andn_iter_no_changeare declared as Cints in thefit_sgd(...)signature (lines 114, 120, 121), so oversized Python values are truncated by Cython before Line 198 executes. The call todims_within_int_limitson lines 198–204 validates the already-truncated values, defeating its purpose as an overflow guard.To make
dims_within_int_limitseffective for these hyperparameters, either:
- Accept them as Python objects here and validate before casting to
int, or- Validate in
SGD.fitbefore callingfit_sgdwith typed parameters.🤖 Prompt for 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. In `@python/cuml/cuml/solvers/sgd.pyx` around lines 111 - 121, The bounds checks are ineffective because fit_sgd(...) currently declares epochs, batch_size, and n_iter_no_change as C ints so Cython has already truncated oversized Python values; update the code so dims_within_int_limits validates the original Python values before coercion—either change fit_sgd's signature to accept those three parameters as PyObject/Python objects (or plain Python ints) and call dims_within_int_limits on them prior to casting to C int, or perform the validation inside SGD.fit before calling fit_sgd with typed ints; reference fit_sgd, SGD.fit, dims_within_int_limits and ensure the validation happens on the uncoerced values and only then cast to int.python/cuml/cuml/metrics/cluster/entropy.pyx (1)
51-57:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCheck integer bounds before coercing
clusteringtonp.int32.On line 51-57,
check_arrayperforms dtype coercion tonp.int32before thevalues_fit_int32validation on line 68-71. This allows overflow to occur silently: values outside the int32 range will wrap/truncate during the conversion, and the subsequent bounds check will validate the corrupted values instead of the original input.Remove
dtype=np.int32from thecheck_arraycall, validate bounds on the original dtype usingvalues_fit_int32, and cast to int32 only after validation succeeds.Proposed fix
clustering = check_array( clustering, ensure_2d=False, order='C', - dtype=np.int32, input_name='clustering', ) if clustering.ndim == 2 and clustering.shape[1] != 1: raise ValueError( "clustering must have shape (n_samples,) or (n_samples, 1), got " f"{clustering.shape}" ) clustering = clustering.ravel() dims_within_int_limits(n_rows=clustering.shape[0]) - cdef int n_rows = clustering.shape[0] lower_class_range = cp.min(clustering).item() upper_class_range = cp.max(clustering).item() values_fit_int32( lower_class_range=lower_class_range, upper_class_range=upper_class_range, ) + clustering = clustering.astype(np.int32, copy=False) + cdef int n_rows = clustering.shape[0]🤖 Prompt for 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. In `@python/cuml/cuml/metrics/cluster/entropy.pyx` around lines 51 - 57, The code currently passes dtype=np.int32 into check_array for the variable clustering which coerces values before bounds checking; remove dtype=np.int32 from the check_array call so clustering is validated in its original dtype, then call values_fit_int32(clustering, 'clustering') to ensure all values fit in int32, and only after that cast/clustering = clustering.astype(np.int32, copy=False) (or equivalent) so conversion happens after successful validation; update references in entropy.pyx around the clustering handling (check_array, values_fit_int32) accordingly.
🤖 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 `@python/cuml/cuml/datasets/regression.pyx`:
- Around line 162-169: The validation for effective_rank currently uses
dims_within_size_t_limits but effective_rank is later cast to long (causing
overflow); replace the call
dims_within_size_t_limits(effective_rank=effective_rank) with a validation that
checks signed-long bounds (e.g., use a dims_within_long_limits helper or
explicitly check effective_rank <= LONG_MAX and >= LONG_MIN) before casting to
long where effective_rank is used; update the validation near the effective_rank
handling in regression.pyx so the cast to long cannot overflow.
In `@python/cuml/cuml/internals/dimension_limits.py`:
- Around line 25-26: Replace uses of int(value) that silently coerce
non-integers with operator.index(value) to enforce the integer protocol: import
operator, then in each guard where you currently do "v = int(value)" (the
occurrences at the four guards in this file) use "v = operator.index(value)"
inside a try/except to let a TypeError propagate or raise a clear TypeError for
non-integer-like inputs, and keep the existing range checks against
INT32_MIN/INT32_MAX afterwards; update the error messages to reflect that
non-integer inputs are rejected.
In `@python/cuml/cuml/manifold/t_sne.pyx`:
- Around line 605-621: The code narrows X_m.nnz into a C int before validating
its size; change to read X_m.nnz into a Python int temporary (e.g., X_nnz_py =
int(X_m.nnz)), call dims_within_int_limits(...) using that Python int (pass
X_nnz=X_nnz_py and csr_indptr_len=n_s+1 when sparse_fit), and only after
validation cast to the native C int (e.g., X_nnz = <int>X_nnz_py) for use later
in fit; update the block around input_to_cuml_array / sparse handling where
X_nnz and dims_within_int_limits are used so validation occurs prior to any C
narrowing.
In `@python/cuml/cuml/svm/linear.pyx`:
- Around line 310-317: The code currently converts shapes to native types
without verifying that scores and prob_scale are compatible for
computeProbabilities; add explicit shape validation before calling
dims_within_size_t_limits/dims_within_int_limits: ensure scores.ndim == 2 and
prob_scale.ndim == 2, that scores.shape[1] == prob_scale.shape[0] (columns of
scores match rows of prob_scale), and that prob_scale.shape[1] == 2; if any
check fails, raise a ValueError with a clear message. Keep these checks adjacent
to the existing shape extraction (the scores, prob_scale block) so n_rows,
n_classes conversion and subsequent calls to
dims_within_size_t_limits/dims_within_int_limits and computeProbabilities
receive validated sizes.
In `@python/cuml/cuml/svm/svm_base.pyx`:
- Around line 527-531: The code currently assigns support_nnz directly to a C
int before running dims_within_int_limits; instead, first read
support_vectors.nnz into a Python int temporary (e.g., py_support_nnz =
support_vectors.nnz), call dims_within_int_limits(support_nnz=py_support_nnz) to
validate bounds, and only then cast to the C variable support_nnz (cdef int
support_nnz = <int>py_support_nnz) inside the sparse_model branch so the
narrowing happens after validation; update the logic around support_nnz,
sparse_model, and the call to dims_within_int_limits accordingly.
---
Outside diff comments:
In `@python/cuml/cuml/cluster/spectral_clustering.pyx`:
- Line 315: Assign Python temporaries for the values that will be stored in C
ints (affinity_nnz, config.n_clusters, config.n_init) and validate them with
dims_within_int_limits before writing into the cdef int fields; follow the same
pattern used for n_samples and n_features (create py_* temporaries, call
dims_within_int_limits on those, then assign to affinity_nnz, config.n_clusters,
config.n_init) to avoid truncation or overflow when populating the C-backed
fields in spectral_clustering.pyx.
In `@python/cuml/cuml/linear_model/base_mg.pyx`:
- Around line 77-78: The current check forces sparse X_nnz through a 32-bit int
bound unconditionally, blocking valid 64-bit sparse inputs; modify the logic
around self.index_dtype (set from X_m.indptr.dtype) so that the 32-bit bound is
only applied when self.index_dtype is a 32-bit integer type, and when
self.index_dtype is 64-bit either skip the 32-bit cast or use a 64-bit-safe
bound (int64_t) before calling the model-specific _fit (e.g.,
LogisticRegressionMG._fit) so 64-bit overloads can be reached; update the
conditional around X_nnz handling to branch on self.index_dtype rather than
always applying the 32-bit path.
In `@python/cuml/cuml/metrics/cluster/entropy.pyx`:
- Around line 51-57: The code currently passes dtype=np.int32 into check_array
for the variable clustering which coerces values before bounds checking; remove
dtype=np.int32 from the check_array call so clustering is validated in its
original dtype, then call values_fit_int32(clustering, 'clustering') to ensure
all values fit in int32, and only after that cast/clustering =
clustering.astype(np.int32, copy=False) (or equivalent) so conversion happens
after successful validation; update references in entropy.pyx around the
clustering handling (check_array, values_fit_int32) accordingly.
In `@python/cuml/cuml/solvers/sgd.pyx`:
- Around line 111-121: The bounds checks are ineffective because fit_sgd(...)
currently declares epochs, batch_size, and n_iter_no_change as C ints so Cython
has already truncated oversized Python values; update the code so
dims_within_int_limits validates the original Python values before
coercion—either change fit_sgd's signature to accept those three parameters as
PyObject/Python objects (or plain Python ints) and call dims_within_int_limits
on them prior to casting to C int, or perform the validation inside SGD.fit
before calling fit_sgd with typed ints; reference fit_sgd, SGD.fit,
dims_within_int_limits and ensure the validation happens on the uncoerced values
and only then cast to int.
🪄 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: ca6b05ca-f6a4-4b2d-81a0-94e1cb9b3dfd
📒 Files selected for processing (50)
python/cuml/cuml/cluster/agglomerative.pyxpython/cuml/cuml/cluster/dbscan.pyxpython/cuml/cuml/cluster/hdbscan/hdbscan.pyxpython/cuml/cuml/cluster/kmeans.pyxpython/cuml/cuml/cluster/spectral_clustering.pyxpython/cuml/cuml/common/opg_data_utils_mg.pyxpython/cuml/cuml/datasets/arima.pyxpython/cuml/cuml/datasets/regression.pyxpython/cuml/cuml/decomposition/pca.pyxpython/cuml/cuml/decomposition/pca_mg.pyxpython/cuml/cuml/decomposition/tsvd.pyxpython/cuml/cuml/decomposition/tsvd_mg.pyxpython/cuml/cuml/ensemble/randomforest_common.pyxpython/cuml/cuml/experimental/linear_model/lars.pyxpython/cuml/cuml/explainer/base.pyxpython/cuml/cuml/explainer/kernel_shap.pyxpython/cuml/cuml/explainer/permutation_shap.pyxpython/cuml/cuml/explainer/tree_shap.pyxpython/cuml/cuml/fil/fil.pyxpython/cuml/cuml/internals/dimension_limits.pypython/cuml/cuml/linear_model/base_mg.pyxpython/cuml/cuml/linear_model/linear_regression.pyxpython/cuml/cuml/linear_model/logistic_regression_mg.pyxpython/cuml/cuml/linear_model/ridge.pyxpython/cuml/cuml/manifold/spectral_embedding.pyxpython/cuml/cuml/manifold/t_sne.pyxpython/cuml/cuml/manifold/umap/umap.pyxpython/cuml/cuml/metrics/cluster/adjusted_rand_index.pyxpython/cuml/cuml/metrics/cluster/entropy.pyxpython/cuml/cuml/metrics/cluster/silhouette_score.pyxpython/cuml/cuml/metrics/cluster/utils.pypython/cuml/cuml/metrics/kl_divergence.pyxpython/cuml/cuml/metrics/pairwise_distances.pyxpython/cuml/cuml/metrics/trustworthiness.pyxpython/cuml/cuml/neighbors/kneighbors_classifier.pyxpython/cuml/cuml/neighbors/kneighbors_classifier_mg.pyxpython/cuml/cuml/neighbors/kneighbors_regressor.pyxpython/cuml/cuml/neighbors/kneighbors_regressor_mg.pyxpython/cuml/cuml/neighbors/nearest_neighbors.pyxpython/cuml/cuml/neighbors/nearest_neighbors_mg.pyxpython/cuml/cuml/solvers/cd.pyxpython/cuml/cuml/solvers/cd_mg.pyxpython/cuml/cuml/solvers/qn.pyxpython/cuml/cuml/solvers/sgd.pyxpython/cuml/cuml/svm/linear.pyxpython/cuml/cuml/svm/svm_base.pyxpython/cuml/cuml/tsa/arima.pyxpython/cuml/cuml/tsa/auto_arima.pyxpython/cuml/cuml/tsa/holtwinters.pyxpython/cuml/cuml/tsa/stationarity.pyx
| dims_within_size_t_limits( | ||
| n_samples=n_samples, | ||
| n_features=n_features, | ||
| n_informative=n_informative, | ||
| n_targets=n_targets, | ||
| ) | ||
| if effective_rank >= 0: | ||
| dims_within_size_t_limits(effective_rank=effective_rank) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python - <<'PY'
import ctypes
long_bits = ctypes.sizeof(ctypes.c_long) * 8
size_t_bits = ctypes.sizeof(ctypes.c_size_t) * 8
print(f"c_long bits: {long_bits}")
print(f"c_size_t bits: {size_t_bits}")
print(f"LONG_MAX: {2 ** (long_bits - 1) - 1}")
print(f"SIZE_T_MAX: {2 ** size_t_bits - 1}")
PY
rg -n -C2 'cpp_make_regression|dims_within_size_t_limits|effective_rank|<long>' \
python/cuml/cuml/datasets/regression.pyxRepository: rapidsai/cuml
Length of output: 3206
🏁 Script executed:
fd -t f "dimension_limits" --type f | head -20Repository: rapidsai/cuml
Length of output: 104
🏁 Script executed:
rg -l "def dims_within_size_t_limits|cdef.*dims_within_size_t_limits" --type py --type pyx --type pxdRepository: rapidsai/cuml
Length of output: 85
🏁 Script executed:
cat -n python/cuml/cuml/datasets/regression.pyx | sed -n '155,210p'Repository: rapidsai/cuml
Length of output: 2540
🏁 Script executed:
cat -n python/cuml/cuml/internals/dimension_limits.pyRepository: rapidsai/cuml
Length of output: 3591
Fix validation bounds mismatch: effective_rank must be validated against long limits, not size_t.
Line 169 validates effective_rank using dims_within_size_t_limits, which checks against the full size_t range (up to 2^64−1 on 64-bit systems). However, the C++ function expects long effective_rank, which on 64-bit systems has a narrower upper bound (2^63−1). Values between these limits pass validation but overflow when cast to <long> at Lines 196 and 205. Use a validation function that checks signed-long bounds instead, or add explicit bounds checking for values > LONG_MAX before the cast.
🤖 Prompt for 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.
In `@python/cuml/cuml/datasets/regression.pyx` around lines 162 - 169, The
validation for effective_rank currently uses dims_within_size_t_limits but
effective_rank is later cast to long (causing overflow); replace the call
dims_within_size_t_limits(effective_rank=effective_rank) with a validation that
checks signed-long bounds (e.g., use a dims_within_long_limits helper or
explicitly check effective_rank <= LONG_MAX and >= LONG_MIN) before casting to
long where effective_rank is used; update the validation near the effective_rank
handling in regression.pyx so the cast to long cannot overflow.
| v = int(value) | ||
| if v < INT32_MIN or v > INT32_MAX: |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the dimension_limits.py file
cat -n python/cuml/cuml/internals/dimension_limits.pyRepository: rapidsai/cuml
Length of output: 3591
🏁 Script executed:
# Check how these functions are called in the codebase
rg -A 3 "values_fit_int32|dims_within_int_limits|dims_within_uint32_limits|dims_within_size_t_limits" --type py | head -100Repository: rapidsai/cuml
Length of output: 2470
🏁 Script executed:
# Check imports and type hints in the module
head -30 python/cuml/cuml/internals/dimension_limits.pyRepository: rapidsai/cuml
Length of output: 1102
🏁 Script executed:
# Find all callers of these guard functions
rg "values_fit_int32|dims_within_int_limits|dims_within_uint32_limits|dims_within_size_t_limits" --type py -B 2 -A 5Repository: rapidsai/cuml
Length of output: 3983
🏁 Script executed:
# Check if there are any type stubs or if these functions are called with explicit types
rg "\.pyi|type: ignore|cast\(" python/cuml/cuml/internals/ --type py -A 2 -B 2Repository: rapidsai/cuml
Length of output: 856
🏁 Script executed:
# Check if there are any tests for these guard functions
fd -e py test | xargs rg "values_fit_int32|dims_within_int_limits|dims_within_uint32_limits|dims_within_size_t_limits" --type pyRepository: rapidsai/cuml
Length of output: 41
🏁 Script executed:
# Check the metrics/cluster/utils.py more carefully to understand caller context
sed -n '1,100p' python/cuml/cuml/metrics/cluster/utils.py | head -60Repository: rapidsai/cuml
Length of output: 2190
Reject non-integer inputs in dimension guards using operator.index().
Using int(value) here can silently truncate non-integral inputs (for example 3.9 → 3) before range checks, which undermines the safety contract of these guard functions. These validators should reject non-integer-like values outright rather than coerce them. Use operator.index() to enforce strict integer protocol.
🔧 Proposed fix
+from operator import index as _index
+
+def _require_index(name: str, value) -> int:
+ try:
+ return _index(value)
+ except TypeError as e:
+ raise ValueError(f"{name} must be an integer, got {value!r}") from e
+
def values_fit_int32(**values: int) -> None:
@@
for name, value in values.items():
- v = int(value)
+ v = _require_index(name, value)
@@
def dims_within_int_limits(**dims: int) -> None:
@@
for name, value in dims.items():
- v = int(value)
+ v = _require_index(name, value)
@@
def dims_within_uint32_limits(**dims: int) -> None:
@@
for name, value in dims.items():
- v = int(value)
+ v = _require_index(name, value)
@@
def dims_within_size_t_limits(**dims: int) -> None:
@@
for name, value in dims.items():
- v = int(value)
+ v = _require_index(name, value)As per coding guidelines: "Silent data corruption from type coercion ... must be addressed."
Also applies to: 43-44, 57-58, 75-76
🤖 Prompt for 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.
In `@python/cuml/cuml/internals/dimension_limits.py` around lines 25 - 26, Replace
uses of int(value) that silently coerce non-integers with operator.index(value)
to enforce the integer protocol: import operator, then in each guard where you
currently do "v = int(value)" (the occurrences at the four guards in this file)
use "v = operator.index(value)" inside a try/except to let a TypeError propagate
or raise a clear TypeError for non-integer-like inputs, and keep the existing
range checks against INT32_MIN/INT32_MAX afterwards; update the error messages
to reflect that non-integer inputs are rejected.
| n_s, n_f = map(int, X_m.shape) | ||
| X_nnz = int(X_m.nnz) | ||
| X_ptr = <uintptr_t>X_m.data.ptr | ||
| X_indptr_ptr = <uintptr_t>X_m.indptr.ptr | ||
| X_indices_ptr = <uintptr_t>X_m.indices.ptr | ||
| X_nnz = X_m.nnz | ||
| else: | ||
| X_m, n_samples, n_features, _ = input_to_cuml_array( | ||
| X_m, n_s, n_f, _ = input_to_cuml_array( | ||
| X, order='F', check_dtype=np.float32, | ||
| convert_to_dtype=(np.float32 if convert_dtype else None) | ||
| ) | ||
| n_s, n_f = int(n_s), int(n_f) | ||
| X_ptr = X_m.ptr | ||
|
|
||
| dims_within_int_limits(n_samples=n_s, n_features=n_f) | ||
| if sparse_fit: | ||
| dims_within_int_limits(X_nnz=X_nnz, csr_indptr_len=n_s + 1) | ||
|
|
There was a problem hiding this comment.
Validate sparse nnz before assigning to native int in fit.
X_nnz is narrowed first and only then checked. Use a Python-int temporary, validate, then cast to cdef int.
Suggested fix
if sparse_fit:
X_m = SparseCumlArray(X, convert_to_dtype=cupy.float32)
n_s, n_f = map(int, X_m.shape)
- X_nnz = int(X_m.nnz)
+ X_nnz_py = int(X_m.nnz)
X_ptr = <uintptr_t>X_m.data.ptr
X_indptr_ptr = <uintptr_t>X_m.indptr.ptr
X_indices_ptr = <uintptr_t>X_m.indices.ptr
else:
@@
dims_within_int_limits(n_samples=n_s, n_features=n_f)
if sparse_fit:
- dims_within_int_limits(X_nnz=X_nnz, csr_indptr_len=n_s + 1)
+ dims_within_int_limits(X_nnz=X_nnz_py, csr_indptr_len=n_s + 1)
+ X_nnz = <int>X_nnz_py📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| n_s, n_f = map(int, X_m.shape) | |
| X_nnz = int(X_m.nnz) | |
| X_ptr = <uintptr_t>X_m.data.ptr | |
| X_indptr_ptr = <uintptr_t>X_m.indptr.ptr | |
| X_indices_ptr = <uintptr_t>X_m.indices.ptr | |
| X_nnz = X_m.nnz | |
| else: | |
| X_m, n_samples, n_features, _ = input_to_cuml_array( | |
| X_m, n_s, n_f, _ = input_to_cuml_array( | |
| X, order='F', check_dtype=np.float32, | |
| convert_to_dtype=(np.float32 if convert_dtype else None) | |
| ) | |
| n_s, n_f = int(n_s), int(n_f) | |
| X_ptr = X_m.ptr | |
| dims_within_int_limits(n_samples=n_s, n_features=n_f) | |
| if sparse_fit: | |
| dims_within_int_limits(X_nnz=X_nnz, csr_indptr_len=n_s + 1) | |
| n_s, n_f = map(int, X_m.shape) | |
| X_nnz_py = int(X_m.nnz) | |
| X_ptr = <uintptr_t>X_m.data.ptr | |
| X_indptr_ptr = <uintptr_t>X_m.indptr.ptr | |
| X_indices_ptr = <uintptr_t>X_m.indices.ptr | |
| else: | |
| X_m, n_s, n_f, _ = input_to_cuml_array( | |
| X, order='F', check_dtype=np.float32, | |
| convert_to_dtype=(np.float32 if convert_dtype else None) | |
| ) | |
| n_s, n_f = int(n_s), int(n_f) | |
| X_ptr = X_m.ptr | |
| dims_within_int_limits(n_samples=n_s, n_features=n_f) | |
| if sparse_fit: | |
| dims_within_int_limits(X_nnz=X_nnz_py, csr_indptr_len=n_s + 1) | |
| X_nnz = <int>X_nnz_py |
🤖 Prompt for 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.
In `@python/cuml/cuml/manifold/t_sne.pyx` around lines 605 - 621, The code narrows
X_m.nnz into a C int before validating its size; change to read X_m.nnz into a
Python int temporary (e.g., X_nnz_py = int(X_m.nnz)), call
dims_within_int_limits(...) using that Python int (pass X_nnz=X_nnz_py and
csr_indptr_len=n_s+1 when sparse_fit), and only after validation cast to the
native C int (e.g., X_nnz = <int>X_nnz_py) for use later in fit; update the
block around input_to_cuml_array / sparse handling where X_nnz and
dims_within_int_limits are used so validation occurs prior to any C narrowing.
| # Extract dimensions (validate before narrowing to native widths) | ||
| n_rows_py = int(scores.shape[0]) | ||
| n_classes_py = int(prob_scale.shape[0]) | ||
| dims_within_size_t_limits(n_rows=n_rows_py) | ||
| dims_within_int_limits(n_classes=n_classes_py) | ||
| cdef size_t n_rows = n_rows_py | ||
| cdef int n_classes = n_classes_py | ||
|
|
There was a problem hiding this comment.
Validate scores and prob_scale shape compatibility before native probability kernel call.
Current code validates integer ranges but not shape agreement (scores.shape[1] vs prob_scale.shape[0], and prob_scale.shape[1] == 2). A mismatch can pass invalid pointers/sizes into computeProbabilities.
Suggested fix
# Ensure proper ordering
prob_scale = cp.asarray(prob_scale, order="F")
scores = cp.asarray(scores, order="C", dtype=prob_scale.dtype)
+if scores.ndim != 2:
+ raise ValueError(f"`scores` must be 2D, got shape={scores.shape}")
+if prob_scale.ndim != 2 or prob_scale.shape[1] != 2:
+ raise ValueError(
+ f"`prob_scale` must have shape (n_classes, 2), got shape={prob_scale.shape}"
+ )
+if scores.shape[1] != prob_scale.shape[0]:
+ raise ValueError(
+ f"Shape mismatch: scores has {scores.shape[1]} classes but "
+ f"prob_scale has {prob_scale.shape[0]} rows"
+ )
+
# Extract dimensions (validate before narrowing to native widths)
n_rows_py = int(scores.shape[0])
n_classes_py = int(prob_scale.shape[0])📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Extract dimensions (validate before narrowing to native widths) | |
| n_rows_py = int(scores.shape[0]) | |
| n_classes_py = int(prob_scale.shape[0]) | |
| dims_within_size_t_limits(n_rows=n_rows_py) | |
| dims_within_int_limits(n_classes=n_classes_py) | |
| cdef size_t n_rows = n_rows_py | |
| cdef int n_classes = n_classes_py | |
| if scores.ndim != 2: | |
| raise ValueError(f"`scores` must be 2D, got shape={scores.shape}") | |
| if prob_scale.ndim != 2 or prob_scale.shape[1] != 2: | |
| raise ValueError( | |
| f"`prob_scale` must have shape (n_classes, 2), got shape={prob_scale.shape}" | |
| ) | |
| if scores.shape[1] != prob_scale.shape[0]: | |
| raise ValueError( | |
| f"Shape mismatch: scores has {scores.shape[1]} classes but " | |
| f"prob_scale has {prob_scale.shape[0]} rows" | |
| ) | |
| # Extract dimensions (validate before narrowing to native widths) | |
| n_rows_py = int(scores.shape[0]) | |
| n_classes_py = int(prob_scale.shape[0]) | |
| dims_within_size_t_limits(n_rows=n_rows_py) | |
| dims_within_int_limits(n_classes=n_classes_py) | |
| cdef size_t n_rows = n_rows_py | |
| cdef int n_classes = n_classes_py |
🤖 Prompt for 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.
In `@python/cuml/cuml/svm/linear.pyx` around lines 310 - 317, The code currently
converts shapes to native types without verifying that scores and prob_scale are
compatible for computeProbabilities; add explicit shape validation before
calling dims_within_size_t_limits/dims_within_int_limits: ensure scores.ndim ==
2 and prob_scale.ndim == 2, that scores.shape[1] == prob_scale.shape[0] (columns
of scores match rows of prob_scale), and that prob_scale.shape[1] == 2; if any
check fails, raise a ValueError with a clear message. Keep these checks adjacent
to the existing shape extraction (the scores, prob_scale block) so n_rows,
n_classes conversion and subsequent calls to
dims_within_size_t_limits/dims_within_int_limits and computeProbabilities
receive validated sizes.
| cdef int support_nnz | ||
| if sparse_model: | ||
| support_nnz = int(support_vectors.nnz) | ||
| dims_within_int_limits(support_nnz=support_nnz) | ||
| else: |
There was a problem hiding this comment.
Run support_nnz bounds validation before narrowing to cdef int.
support_nnz is currently assigned as a native int before the guard runs. Validate with a Python-int temporary first, then cast after validation.
Suggested fix
cdef int support_nnz
if sparse_model:
- support_nnz = int(support_vectors.nnz)
- dims_within_int_limits(support_nnz=support_nnz)
+ support_nnz_py = int(support_vectors.nnz)
+ dims_within_int_limits(support_nnz=support_nnz_py)
+ support_nnz = <int>support_nnz_py
else:
support_nnz = -1🤖 Prompt for 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.
In `@python/cuml/cuml/svm/svm_base.pyx` around lines 527 - 531, The code currently
assigns support_nnz directly to a C int before running dims_within_int_limits;
instead, first read support_vectors.nnz into a Python int temporary (e.g.,
py_support_nnz = support_vectors.nnz), call
dims_within_int_limits(support_nnz=py_support_nnz) to validate bounds, and only
then cast to the C variable support_nnz (cdef int support_nnz =
<int>py_support_nnz) inside the sparse_model branch so the narrowing happens
after validation; update the logic around support_nnz, sparse_model, and the
call to dims_within_int_limits accordingly.
No description provided.