Skip to content

Python interface + benchmarks for BitBIRCH - #377

Merged
scal444 merged 3 commits into
NVIDIA-BioNeMo:mainfrom
scal444:bitbirch-python
Oct 26, 2026
Merged

scal444 merged 3 commits into
NVIDIA-BioNeMo:mainfrom
scal444:bitbirch-python

Conversation

@scal444

@scal444 scal444 commented Oct 23, 2026

Copy link
Copy Markdown
Collaborator

No description provided.

nvmolkit.clustering.bitbirch clusters packed int32 or uint32 fingerprints
from any array input and returns labels, plus centroids on request. Tests
compare labels and centroids with an independent CPU model of the batched
schedule (nvmolkit/tests/_bitbirch_reference.py), whose trees are audited,
across thresholds, branching factors up to 2048, multi-tile cascading
splits, grouped merges past the ordered warm-up, and 4096-bit rows.
benchmarks/bitbirch_clustering_bench.py times BitBIRCH on real molecules,
optionally against bblean.
@greptile-apps

greptile-apps Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The changes appear safe to merge; no new actionable issues were found.

Summary

Adds a Python interface for GPU BitBIRCH clustering, optional majority centroids, benchmarks, and reference-based tests.

  • Python callers can cluster packed fingerprints with GPU BitBIRCH.
  • A new script compares GPU and CPU BitBIRCH runs on molecular fingerprints.
  • Clustering docs put method citations beside each API.

Reviews (3) · Last reviewed commit: "Address BitBIRCH review: report nvMolKit..." · Reviewed by Greptile

Comment thread docs/index.rst Outdated
@scal444
scal444 requested a review from evasnow1992 October 23, 2026 13:41
)
configuration_rows.append(
{
"method": "nvmolkit",

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.

The bblean benchmark reports the full cluster-size distribution, while the nvMolKit benchmark only reports num_clusters. Since the two implementations may produce different clusterings due to their insertion strategies, could we also call _cluster_distribution on the nvMolKit labels? This would make the throughput comparison more informative.

Comment thread nvmolkit/clustering.py Outdated
x: Shape ``(N, W)`` packed int32 or uint32 fingerprints, ``W <= 128``
(at most 4096 bits). Host arrays are copied to the GPU.
threshold: Minimum combined-cluster iSIM Jaccard--Tanimoto similarity.
branching_factor: Maximum entries per tree node. Must be at least 3.

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.

The C++ implementation also has an upper limit of 65,534 for branching_factor, but the Python docstring only mentions the lower bound of 3. Could we document and validate this limit here? Otherwise, users exceeding it receive a generic error message.

@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.

Two minor comments. Changes look good to me

…branching_factor

The benchmark now reports the same cluster-size distribution for nvMolKit as for
bblean, so throughput comparisons show whether the clusterings differ.
bitbirch() validates and documents the 65534 branching_factor ceiling enforced
by the C++ implementation instead of surfacing its generic error.
@scal444
scal444 merged commit 9895e90 into NVIDIA-BioNeMo:main Oct 26, 2026
15 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.

2 participants