Skip to content

Fix dead RDKIT_NEW_FLAG_API gate in ETKDG chirality and bounds code#205

Merged
scal444 merged 1 commit into
NVIDIA-BioNeMo:mainfrom
scal444:issue-202-embed-failure
Jun 12, 2026
Merged

Fix dead RDKIT_NEW_FLAG_API gate in ETKDG chirality and bounds code#205
scal444 merged 1 commit into
NVIDIA-BioNeMo:mainfrom
scal444:issue-202-embed-failure

Conversation

@scal444

@scal444 scal444 commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator

GH issue 202: a non-negligible fraction of molecules embedded to zero conformers. RDKIT_NEW_FLAG_API was a constexpr bool but consumed with #if, which the preprocessor evaluates as 0, and etkdg_stages and rdkit_dist_geom_flattened never linked nvmolkit_versions so versions.h was not even visible. The gate was therefore always off.

findChiralSets never set d_structureFlags and loadChiralDataset never read it, so fused-small-ring tetrahedral centers used volScale 1.0 instead of the 0.25 RDKit applies and were deterministically rejected at the tetrahedral check. The same gate guards the constrained tight-bounds branch in addLongRangeDistanceConstraints, which assigns to l and u; those are now non-const so the branch compiles.

GH issue 202: a non-negligible fraction of molecules embedded to zero
conformers. RDKIT_NEW_FLAG_API was a constexpr bool but consumed with #if,
which the preprocessor evaluates as 0, and etkdg_stages and
rdkit_dist_geom_flattened never linked nvmolkit_versions so versions.h was
not even visible. The gate was therefore always off.

findChiralSets never set d_structureFlags and loadChiralDataset never read
it, so fused-small-ring tetrahedral centers used volScale 1.0 instead of the
0.25 RDKit applies and were deterministically rejected at the tetrahedral
check. The same gate guards the constrained tight-bounds branch in
addLongRangeDistanceConstraints, which assigns to l and u; those are now
non-const so the branch compiles.
@scal444 scal444 requested a review from evasnow1992 June 11, 2026 21:07
@scal444

scal444 commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator Author

Fixes #202

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

@scal444 scal444 merged commit 525286a into NVIDIA-BioNeMo:main Jun 12, 2026
9 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