Skip to content

Uff python bindings#129

Merged
scal444 merged 6 commits intoNVIDIA-Digital-Bio:mainfrom
scal444:uff_python
Apr 14, 2026
Merged

Uff python bindings#129
scal444 merged 6 commits intoNVIDIA-Digital-Bio:mainfrom
scal444:uff_python

Conversation

@scal444
Copy link
Copy Markdown
Collaborator

@scal444 scal444 commented Apr 7, 2026

No description provided.

@scal444 scal444 requested a review from evasnow1992 April 7, 2026 16:51
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 7, 2026

Greptile Summary

This PR adds GPU-accelerated UFF force field optimization mirroring the existing MMFF implementation, and takes the opportunity to consolidate previously duplicated utility code (vectorToList, extractMolecules, ThreadLocalBuffers, etc.) into shared headers (boost_python_utils.h, mmff_python_utils.h, bfgs_common.h/cpp). The refactoring is clean and the new UFF path is structurally sound.

Confidence Score: 5/5

  • Safe to merge; the one finding is a defensive-coding suggestion with no current impact.
  • No P0 or P1 issues found. The only comment is a P2 on the newly shared extractMMFFPropertiesList being more lenient than the old strict check, but the Python wrapper always supplies exactly numMols entries so the looser C++ validation has no real-world impact. Prior concerns (Any-typed stubs, unchecked EmbedMultipleConfs) are resolved.
  • No files require special attention.

Important Files Changed

Filename Overview
src/minimizer/bfgs_uff.cpp New UFF BFGS minimizer; mirrors bfgs_mmff.cpp closely with correct per-molecule vdwThreshold/ignoreInterfrag indexing via confInfo.molIdx. Uses shared bfgs_common infrastructure. No issues found.
src/minimizer/bfgs_common.cpp New shared BFGS infrastructure extracted from bfgs_mmff.cpp. ensureCapacity now uses energiesSize1.3 for the energy buffer (previously used positionsSize1.3), which is a correct improvement. Thread-safety for writeBackResults is preserved.
nvmolkit/uffOptimization.py Python wrapper with solid input validation (None + UFFHasAllMoleculeParams checks) and scalar/sequence normalization. Mirrors mmffOptimization.py structure. No issues found.
nvmolkit/boost_python_utils.h New shared header consolidating previously duplicated vectorToList, vectorOfVectorsToList, extractMolecules, extractDoubleList, extractBoolList helpers from multiple .cpp files. Clean refactoring.
nvmolkit/mmff_python_utils.h New header for MMFF Python utilities. extractMMFFPropertiesList silently fills missing entries with defaults and ignores excess ones — lenient behavior vs. the strict equality check that was previously in mmffOptimization.cpp. Python wrapper always supplies exactly numMols items, so in practice this is benign.

Reviews (3): Last reviewed commit: "Merge branch 'main' into uff_python" | Re-trigger Greptile

Comment thread nvmolkit/_uffOptimization.pyi Outdated
Comment thread nvmolkit/tests/test_uff_optimization.py Outdated
Comment thread src/minimizer/bfgs_uff.cpp
Comment thread nvmolkit/uffOptimization.cpp Outdated
Comment thread nvmolkit/uffOptimization.cpp Outdated
Copy link
Copy Markdown
Collaborator

@evasnow1992 evasnow1992 left a comment

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. Two minor commends regarding code duplication. Also we are missing updating docs/index.rst and docs/api/nvmolkit.rst with UFF. Would prefer to have them together with the current PR but not required.

@scal444
Copy link
Copy Markdown
Collaborator Author

scal444 commented Apr 14, 2026

Updated docs

@scal444 scal444 merged commit c6c671e into NVIDIA-Digital-Bio:main Apr 14, 2026
10 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