Repository navigation
Python interface + benchmarks for BitBIRCH - #377
Conversation
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.
[Medium impact] The changes appear safe to merge; no new actionable issues were found.
|
| ) | ||
| configuration_rows.append( | ||
| { | ||
| "method": "nvmolkit", |
There was a problem hiding this comment.
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.
| 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. |
There was a problem hiding this comment.
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
left a comment
There was a problem hiding this comment.
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.
No description provided.