Skip to content

Deduplicate Recursive SMARTS Solves - #313

Merged
scal444 merged 2 commits into
NVIDIA-BioNeMo:mainfrom
scal444:codex/dedup-recursive-smarts
Sep 22, 2026
Merged

scal444 merged 2 commits into
NVIDIA-BioNeMo:mainfrom
scal444:codex/dedup-recursive-smarts

Conversation

@scal444

@scal444 scal444 commented Sep 18, 2026

Copy link
Copy Markdown
Collaborator

When the same recursive SMARTS occurs multiple times at the same level, we should solve it once and broadcast rather than solving multiple times. We do that simply by mapping the pattern to multiple output bits. If that ever becomes a problem we could alias bits.

@greptile-apps

greptile-apps Bot commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

The PR appears safe to merge, with no outstanding correctness, security, or repository-rule issues identified.

Summary

The PR deduplicates equivalent recursive SMARTS patterns at each nesting depth, evaluates each unique pattern once, and broadcasts its result through a combined bit mask.

  • Uses depth-qualified SMARTS strings to identify duplicate patterns.
  • Replaces single pattern IDs with masks covering equivalent occurrences.
  • Reuses precomputed per-query pattern metadata during mini-batch preprocessing.
  • Adds preprocessing and end-to-end matching coverage for repeated flat and nested recursive patterns.

Reviews (3) · Last reviewed commit: "Remove unused recursive preprocessor par..."

@scal444
scal444 force-pushed the codex/dedup-recursive-smarts branch from 07315b6 to c7a9ff4 Compare September 18, 2026 22:18
@scal444
scal444 requested a review from evasnow1992 September 19, 2026 09:13
Comment thread src/substruct/recursive_preprocessor.cu Outdated

@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 add the deduplication. Changes look good to me. One comment for cleaning up an unused parameter.

@scal444
scal444 merged commit 5c7a83a into NVIDIA-BioNeMo:main Sep 22, 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