Repository navigation
Full ETKDG v4 support - #388
Merged
Merged
Conversation
nvMolKit now accepts ETversion 4 on RDKit 2026.09 and later, with either refinement and on every BFGS backend and precision. The ETK and all-in-one kernels already evaluate Gaussian experimental torsions, so embedding only needs RDKit's ETKDGv4 setup: prepareEmbedderArgs selects the Gaussian force constants for the chosen refinement before building the torsions, which RDKit scales by them, and then fills the lookup tables. Tests check that the prepared ETKDGv4 details carry RDKit's force constants and torsion scalings for each refinement, embed small molecules and one over 128 atoms with ETKDGv4 on the batched, per-molecule and hybrid backends with both refinements and precisions (checking bond lengths, with batches that let HYBRID reach the per-molecule kernels), and compare all-in-one ETKDGv4 minimization with RDKit on both backends. The Gaussian kernel tests now prepare their molecules through prepareEmbedderArgs, and the Python embedding tests cover ETKDGv4 with both refinements and its rejection on older RDKit.
The pipeline energy tests compared nvMolKit with RDKit only for the ETKDG presets' default refinement, which is legacy for every preset before ETKDGv4, and neither the C++ nor the Python comparisons included ETKDGv4. The C++ pipeline energy fixture now sets the refinement explicitly and runs each preset with both legacy and all-in-one refinement on RDKit 2026.03.4 and later, and adds ETKDGv4 on RDKit 2026.09 and later. The Python RDKit comparisons, serial and batched, add ETKDGv4 with both refinements and skip it on older RDKit.
Contributor
 [Medium impact] The PR appears safe to merge; no new actionable issues were found.
|
evasnow1992
reviewed
Oct 30, 2026
| - embedFragmentsSeparately is not supported. All fragments will be embedded together. | ||
| - Eigenvalue-based initialization supports at most 256 atoms per molecule. | ||
| - Only ETversion 1 and 2 are supported, so the ETKDGv4 preset raises ValueError. | ||
| - ETKDG versions 1 through 4 are supported. ETKDGv4 requires RDKit 2026.09 or later. |
Collaborator
There was a problem hiding this comment.
We don't support ETKDG version 3 yet, right? Saying "versions 1 through 4" could be misleading.
Collaborator
Author
There was a problem hiding this comment.
We support it all, the 1-2 thing was that there is no "ET" v3, ETKDGv3 usese ET v2. I'm rewording to just say we support it all.
evasnow1992
approved these changes
Oct 30, 2026
evasnow1992
left a comment
evasnow1992
left a comment
Collaborator
There was a problem hiding this comment.
The changes look good to me overall. Just one minor suggestion for the docstring update.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Wires up our new kernels for AIO and Gaussian torsions.
Closes #365, #366, #287