Skip to content

Screen SubstructLibrary queries with pattern fingerprints - #361

Merged
scal444 merged 1 commit into
NVIDIA-BioNeMo:mainfrom
scal444:substructlib-4-pattern-screen
Oct 8, 2026
Merged

scal444 merged 1 commit into
NVIDIA-BioNeMo:mainfrom
scal444:substructlib-4-pattern-screen

Conversation

@scal444

@scal444 scal444 commented Oct 8, 2026

Copy link
Copy Markdown
Collaborator

This matches RDKit's pattern fingerprint prescreening, with a ~5-6x speedup.

Final PR for #350

@scal444
scal444 force-pushed the substructlib-4-pattern-screen branch from 264b773 to 53befc5 Compare October 8, 2026 16:48
@greptile-apps

greptile-apps Bot commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Medium risk] Adds pattern fingerprint screening to substructure search.

The PR appears safe to merge; no new actionable defect was established.

What we checked:

  • Selected targets keep their results: The planner stores original target indexes. Result collection reads those indexes rather than the candidate’s position.
  • Recursive queries keep contiguous targets: Queries with recursive patterns turn off selected-target batching. After matching, the caller clears results for targets outside the candidate list.
Summary

Adds default-on RDKit pattern fingerprint screening to SubstructLibrary, with an option to disable it.

  • Stores fingerprints alongside finalized molecules and builds GPU bit slices.
  • Searches selected targets for ordinary queries. Recursive queries search contiguous targets and then keep candidate results.
  • Adds benchmark controls and tests for screening, selected indexes, and repeated finalize() calls.
  • The earlier freed-pointer concern is addressed by reserving fingerprints_ before committing replacement targets.
  • scal444 accepted the extra pinned host-memory cost because GPU memory already limits query slots and a host limit would complicate the pinned-buffer path. That finding remains withdrawn.

Reviews (3) · Last reviewed commit: "Screen SubstructLibrary queries with pat..." · Reviewed by Greptile

Comment thread src/substruct/substruct_library.cpp
Comment thread src/substruct/pattern_screen.cu
@scal444
scal444 force-pushed the substructlib-4-pattern-screen branch from 53befc5 to 851acf6 Compare October 8, 2026 17:04
@scal444
scal444 requested a review from evasnow1992 October 8, 2026 17:25
Keep an RDKit pattern fingerprint for every molecule and, before the exact
search, screen each query on the GPU the way RDKit's PatternHolder does: a
molecule passes only if it has at least as many atoms as the query and
carries every query fingerprint bit. Each GPU stores its molecules'
fingerprints transposed, one bitmap over molecules per fingerprint bit, so
the screen ANDs the query's bitmaps rarest bit first and stops once no
molecule in a 32-molecule word survives.

Only molecules that pass are searched: hasSubstructMatch() takes an
optional list of candidate targets, planned by the new
MiniBatchPlanner::prepareSelectedTargetsMiniBatch(). Queries with
recursive SMARTS still search every target, since recursive-pattern
matching needs contiguous target ranges. Molecules matched with RDKit are
screened on the CPU with the same fingerprints. The screen is on by
default and never changes results; usePatternFingerprints=false turns it
off, as does the benchmark's --no_pattern_fingerprints flag.
@scal444
scal444 force-pushed the substructlib-4-pattern-screen branch from 851acf6 to f113a3c Compare October 8, 2026 17:28

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

Changes look good to me.

@scal444
scal444 merged commit c097596 into NVIDIA-BioNeMo:main Oct 8, 2026
16 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