Skip to content

Unify Butina outputs for device and RDKit compatibility mode. - #285

Merged
scal444 merged 6 commits into
NVIDIA-BioNeMo:mainfrom
scal444:fix/issue-269-butina-output
Sep 10, 2026
Merged

Unify Butina outputs for device and RDKit compatibility mode. #285
scal444 merged 6 commits into
NVIDIA-BioNeMo:mainfrom
scal444:fix/issue-269-butina-output

Conversation

@scal444

@scal444 scal444 commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Addresses #269 , though with another API modification. There's no good way to unify fused/nonfused Butina + our previous history of return types without a clean approach.

@greptile-apps

greptile-apps Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR unifies the output contracts of the matrix-based and fused Butina clustering APIs.

  • Adds explicit device-resident and RDKit-compatible output modes.
  • Introduces a structured device result containing cluster IDs, centroids, and cluster sizes.
  • Optimizes RDKit cluster reconstruction while preserving deterministic member ordering.
  • Updates tests, API documentation, usage guidance, and benchmarks for the new contract.

Confidence Score: 5/5

The PR appears safe to merge, with no outstanding correctness, security, or repository-rule issues identified.

The recent cluster-size and RDKit-conversion changes preserve CUDA stream ordering, tensor ownership, contiguous cluster mappings, and deterministic member order; no blocking failure remains.

Important Files Changed

Filename Overview
nvmolkit/clustering.py Introduces unified output modes and structured device results, with stream-safe cluster-size calculation and RDKit-compatible host conversion.
nvmolkit/tests/test_clustering.py Updates existing callers and adds coverage for output modes, cluster sizes, exact RDKit compatibility, and member ordering.
skills/nvmolkit-usage/SKILL.md Updates the Butina workflow example and explains how to consume device and RDKit output representations.
benchmarks/butina_clustering_bench.py Selects device output explicitly so the benchmark continues measuring the intended GPU-oriented path.
docs/api/nvmolkit.rst Adds the new output enum and device-result class to generated API documentation.

Reviews (2): Last reviewed commit: "Address Butina review feedback" | Re-trigger Greptile

@scal444

scal444 commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator Author

/nvskills-ci

2 similar comments
@ohadmo

ohadmo commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

/nvskills-ci

@ohadmo

ohadmo commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

/nvskills-ci

Comment thread skills/nvmolkit-usage/SKILL.md Outdated
Comment thread nvmolkit/clustering.py Outdated
Comment thread nvmolkit/clustering.py Outdated

@evasnow1992 evasnow1992 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for consolidating butina outputs and addressing the comments. Changes look good to me.

@scal444
scal444 merged commit 1be4736 into NVIDIA-BioNeMo:main Sep 10, 2026
12 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants