Skip to content

Apply new input validation to cuml.metrics.cluster - #8059

Merged
rapids-bot[bot] merged 4 commits into
NVIDIA:mainfrom
csadorf:issue-7998-apply-new-validation-to-metrics-cluster
May 6, 2026
Merged

Apply new input validation to cuml.metrics.cluster#8059
rapids-bot[bot] merged 4 commits into
NVIDIA:mainfrom
csadorf:issue-7998-apply-new-validation-to-metrics-cluster

Conversation

@csadorf

@csadorf csadorf commented May 6, 2026

Copy link
Copy Markdown
Contributor

Apply new input validation to cuml.metrics.cluster.

Part of #7998

csadorf added 4 commits May 6, 2026 18:44
Replace input_to_cuml_array with check_array / check_consistent_length
from cuml.internals.validation.  Device pointers are now accessed via
.data.ptr on the returned cupy arrays.

ensure_min_samples=0 preserves the existing behaviour for empty inputs
(exercised by test_adjusted_rand_score_small).

xref NVIDIA#7998
Replace input_to_cupy_array with check_array from
cuml.internals.validation.  The check now accepts 1-D or (n, 1) inputs
and rejects anything wider, matching the legacy check_cols=1 behaviour.

xref NVIDIA#7998
Replace input_to_cuml_array in prepare_cluster_metric_inputs (utils.py)
with check_array / check_consistent_length from
cuml.internals.validation.  The helper now returns plain cupy int32
ndarrays instead of CumlArrays, so the four Cython callers
(v_measure, mutual_info_score, homogeneity_score, completeness_score)
are updated to access the device pointer via .data.ptr rather than .ptr.

ensure_min_samples=0 is used throughout to preserve the existing
behaviour for empty inputs.

xref NVIDIA#7998
Replace input_to_cuml_array with check_array / check_consistent_length
from cuml.internals.validation in _silhouette_coeff.  Device pointers
are now accessed via .data.ptr on the returned cupy arrays.

Pass ensure_all_finite=False for the sil_scores output buffer: it is a
pre-allocated cp.empty array that may contain uninitialised values
(including NaN) before the C++ kernel writes into it, so checking for
finite values up-front would raise a spurious ValueError.

xref NVIDIA#7998
@csadorf
csadorf requested a review from a team as a code owner May 6, 2026 18:47
@csadorf
csadorf requested a review from divyegala May 6, 2026 18:47
@github-actions github-actions Bot added the Cython / Python Cython or Python issue label May 6, 2026
@csadorf csadorf added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change and removed Cython / Python Cython or Python issue labels May 6, 2026
@csadorf csadorf changed the title Apply new input validation to cuml.metrics.cluster Apply new input validation to cuml.metrics.cluster May 6, 2026
@coderabbitai

coderabbitai Bot commented May 6, 2026

Copy link
Copy Markdown

Caution

Review failed

Failed to post review comments

📝 Walkthrough

Summary by CodeRabbit

  • Refactor
    • Enhanced clustering metrics with improved input validation and more reliable data processing for adjusted Rand index, completeness score, entropy, homogeneity, mutual information, silhouette coefficient, and V-measure calculations to ensure more consistent behavior across different input formats.

Walkthrough

This PR refactors clustering metrics modules to replace legacy input-handling utilities with standardized NumPy-based validation (check_array, check_consistent_length) and updates pointer extraction to use the .data.ptr pattern across eight files in the cluster metrics module.

Changes

Clustering Metrics Validation Refactor

Layer / File(s) Summary
Import and Utility Updates
python/cuml/cuml/metrics/cluster/adjusted_rand_index.pyx, entropy.pyx, silhouette_score.pyx, utils.py
Removed legacy input_to_cuml_array and input_to_cupy_array imports; added check_array, check_consistent_length, and related validation utilities.
Data Validation Layer
adjusted_rand_index.pyx, entropy.pyx, silhouette_score.pyx, utils.py
Inputs validated via check_array with explicit dtype enforcement (int32 for labels), 1D/shape assertion, and length consistency checks. Replaces previous input conversion paths with explicit validation before backend calls.
Label Preparation and Monotonic Mapping
silhouette_score.pyx, utils.py
Monotonic label derivation reworked; uses cp.unique or make_monotonic with copy=True to produce new validated arrays. Class ranges computed from unique labels with lower bound fixed at 0.
Pointer Extraction Modernization
completeness_score.pyx, homogeneity_score.pyx, mutual_info_score.pyx, silhouette_score.pyx, v_measure.pyx, adjusted_rand_index.pyx, entropy.pyx
Data pointers retrieved via .data.ptr instead of .ptr; updated across all files to align with new array object structure before C++ backend calls.
Backend Integration
adjusted_rand_index.pyx, entropy.pyx, silhouette_score.pyx
Silhouette, ARI, and entropy backend calls updated to accept validated array pointers and shape parameters (n_rows, n_cols); sil_scores buffer handling adds NULL pointer support for optional inputs.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

  • rapidsai/cuml#8050: Applies the same check_array validation refactor to pairwise_kernels metrics.
  • rapidsai/cuml#8044: Performs the same input_to_* to check_array migration pattern in regression metrics modules.
  • rapidsai/cuml#8012: Migrates input_to_cupy_array to check_array validation utilities across additional metrics modules.

Suggested labels

Cython / Python, improvement, non-breaking

Suggested reviewers

  • divyegala
  • jcrist
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and specifically describes the main change: applying new input validation to the cuml.metrics.cluster module, which aligns with the changeset across all modified metric files.
Description check ✅ Passed The description is concisely related to the changeset, referencing the application of new input validation to cuml.metrics.cluster and linking to the parent issue #7998.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@jcrist

jcrist commented May 6, 2026

Copy link
Copy Markdown
Member

/merge

@rapids-bot
rapids-bot Bot merged commit 83cd3f8 into NVIDIA:main May 6, 2026
103 checks passed
@csadorf
csadorf deleted the issue-7998-apply-new-validation-to-metrics-cluster branch May 7, 2026 19:18
@coderabbitai coderabbitai Bot mentioned this pull request May 7, 2026
aminehd added a commit to aminehd/cuml that referenced this pull request Jul 9, 2026
…IA#7199)

homogeneity/completeness/v_measure/mutual_info must be invariant to a
permutation of label values, for any int/float input dtype. int64 labels
(the cp.array default) were passed to C++ funcs taking `const int*` (int32)
and misread, giving wrong, permutation-variant scores. Already fixed on main
by NVIDIA#8059 and NVIDIA#8269 (which force int32); this adds the missing regression test.

Signed-off-by: Amineh Dadsetan <amineh.dadsetan@gmail.com>
rapids-bot Bot pushed a commit that referenced this pull request Jul 10, 2026
…7199) (#8355)

Regression test for #7199.

**Problem:** `homogeneity_score` / `completeness_score` / `v_measure_score` /
`mutual_info_score` were not invariant to permuting label values —
`metric(y, 1 - y_pred) != metric(y, y_pred)`.

**Cause:** int64 labels (the `cp.array([...])` default) were passed to C++
functions that take `const int*` (int32), so the bytes were misread → garbage
labels → wrong scores. float labels were rejected outright.

**Status:** already fixed on `main` by #8059 and #8269 (they force int32 in
`prepare_cluster_metric_inputs`), but no existing cluster-metric test used
anything other than int32, so the bug was never caught.

**This PR** adds `test_cluster_metric_label_permutation_invariance`: for each
of the four metrics it asserts the score is invariant to a label permutation
and matches scikit-learn, across int32 / int64 / float32 inputs. Fails on the
code before #8059/#8269, passes after.

Authors:
  - https://github.com/aminehd

Approvers:
  - Victor Lafargue (https://github.com/viclafargue)

URL: #8355
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

improvement Improvement / enhancement to an existing function non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants