Skip to content

Speed up GETAWAY ITH/ISH clustering - #379

Merged
scal444 merged 1 commit into
NVIDIA-BioNeMo:mainfrom
scal444:perf-campaign-getaway-ith_clustering
Oct 26, 2026
Merged

scal444 merged 1 commit into
NVIDIA-BioNeMo:mainfrom
scal444:perf-campaign-getaway-ith_clustering

Conversation

@scal444

@scal444 scal444 commented Oct 23, 2026

Copy link
Copy Markdown
Collaborator

ITH and ISH group the rounded heavy-atom leverages with RDKit's clusterArray2, a greedy scan comparing the printed values with IsClose2. Lane 0 used to run the scan alone, and each leverage's printed text was built in character arrays that lived in local memory.

  • The warp now runs the scan a cluster at a time: all lanes test up to 32 followers of the cluster's front at once and a ballot finds the first one that is not close.
  • Cluster keys (leading zeros, length and leading integer of the printed fraction) are derived from the rounded significand's digits directly, with 32-bit division when the significand fits, so no text is built.
  • exactPowerOfTen() multiplies binary-digit factors instead of indexing a table, which also lived in local memory.

The clustering follows RDKit's exactly as before, so results are unchanged.

ITH and ISH group the rounded heavy-atom leverages with RDKit's clusterArray2,
a greedy scan comparing the printed values with IsClose2. Lane 0 used to run
the scan alone, and each leverage's printed text was built in character
arrays that lived in local memory.

- The warp now runs the scan a cluster at a time: all lanes test up to 32
  followers of the cluster's front at once and a ballot finds the first one
  that is not close.
- Cluster keys (leading zeros, length and leading integer of the printed
  fraction) are derived from the rounded significand's digits directly, with
  32-bit division when the significand fits, so no text is built.
- exactPowerOfTen() multiplies binary-digit factors instead of indexing a
  table, which also lived in local memory.

The clustering follows RDKit's exactly as before, so results are unchanged.
@greptile-apps

greptile-apps Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge; no actionable issues were found.

Summary

Speeds up GETAWAY’s ITH and ISH calculations without changing the scan’s grouping behavior.

  • GETAWAY finds leverage clusters across the warp.

Review found no actionable issues. Builds and tests were not run.

Reviews (1) · Last reviewed commit: "Speed up GETAWAY ITH/ISH clustering" · Reviewed by Greptile

@scal444
scal444 requested a review from evasnow1992 October 23, 2026 16:13

@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 the optimization! Changes look good to me.

@scal444
scal444 merged commit 3068410 into NVIDIA-BioNeMo:main Oct 26, 2026
14 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