Skip to content

Adds support for reordering=False in Butina clustering#220

Open
Matthew-Neba wants to merge 8 commits into
NVIDIA-BioNeMo:mainfrom
Matthew-Neba:main
Open

Adds support for reordering=False in Butina clustering#220
Matthew-Neba wants to merge 8 commits into
NVIDIA-BioNeMo:mainfrom
Matthew-Neba:main

Conversation

@Matthew-Neba

@Matthew-Neba Matthew-Neba commented Jul 3, 2026

Copy link
Copy Markdown

Summary

Adds support for reordering=False in Butina clustering. Fixes #192

Changes

  • Added a reordering option to the Python and C++ Butina APIs.
  • Implemented fixed-order Butina clustering for reordering=False.
  • Added tests and ensured they passed
  • Updated the benchmark to optionally run nvMolKit Butina with reordering enabled, disabled, or both.

Signed-off-by: Matthew Neba <mattneba4343@gmail.com>
Signed-off-by: Matthew Neba <mattneba4343@gmail.com>
@scal444

scal444 commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the contribution! This looks great, and we can definitely get it in for v0.6. A few comments, mostly minor.

Comment thread src/butina.cu Outdated
Comment thread nvmolkit/clustering.cpp Outdated
Comment thread nvmolkit/clustering.py Outdated
Comment thread src/butina.cu
Signed-off-by: Matthew Neba <mattneba4343@gmail.com>
Signed-off-by: Matthew Neba <mattneba4343@gmail.com>
Signed-off-by: Matthew Neba <mattneba4343@gmail.com>
Signed-off-by: Matthew Neba <mattneba4343@gmail.com>
Signed-off-by: Matthew Neba <mattneba4343@gmail.com>
@Matthew-Neba

Copy link
Copy Markdown
Author

Thanks for all the help! I have fixed the issues you pointed out. I have managed to parallelize that kernel and fixed the other minor issues aswell. I could indeed use the blockReduce idea you mentioned and it works well.

@Matthew-Neba Matthew-Neba requested a review from scal444 July 8, 2026 06:09
@scal444

scal444 commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

Looks great! Can you install ruff 0.15.8 and clang-format-17, and then run admin/run_clang_format.sh and admin/run_ruff_format.sh to get it to pass the linters?

@scal444

scal444 commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

Or i can do it and push a commit if you're ok with that

Signed-off-by: Matthew Neba <mattneba4343@gmail.com>
@Matthew-Neba

Copy link
Copy Markdown
Author

No worries, I ran the formatters. I also fixed an issue that was leading to the forge build failing. Hopefully all loose ends are tied now.

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.

Non-reordering Butina clustering

2 participants