Skip to content

Comments

fix(builder): Generate All Three Inversion Terms per Planar Center#22

Merged
TKanX merged 12 commits intomainfrom
bugfix/21-inversion-terms-collapse-to-single-instance-per-planar-center
Feb 13, 2026
Merged

fix(builder): Generate All Three Inversion Terms per Planar Center#22
TKanX merged 12 commits intomainfrom
bugfix/21-inversion-terms-collapse-to-single-instance-per-planar-center

Conversation

@TKanX
Copy link
Member

@TKanX TKanX commented Feb 13, 2026

Summary:

Corrected the topology builder to generate three distinct inversion terms for each planar center, adhering to the DREIDING force field specification. Previously, the builder collapsed these terms into a single ImproperDihedral, which under-constrained the planarity of sp2 centers. The new implementation explicitly generates an inversion for each neighbor acting as the unique axis, ensuring robust planarity enforcement. Additionally, the topology terminology has been updated to Torsion and Inversion to better reflect standard force field nomenclature.

Changes:

  • Renamed Topology Terms:

    • Renamed ProperDihedral to Torsion and ImproperDihedral to Inversion in core::topology and throughout the codebase.
    • Updated documentation to reflect these standard terms.
  • Corrected Inversion Generation Logic:

    • Refactored build_inversions (formerly build_improper_dihedrals) to iterate through all three neighbors of a planar center.
    • For a center I with neighbors J, K, L, the builder now emits three distinct terms: I-J-KL, I-K-JL, and I-L-JK.
    • Updated the Inversion constructor to sort only the two plane atoms, preserving the unique axis atom to distinguish the three terms.
  • Updated Tests:

    • Modified unit tests in builder/mod.rs to verify that three inversion terms are generated for a planar center instead of one.
    • Updated integration tests to expect torsions instead of proper_dihedrals and inversions instead of improper_dihedrals.

@TKanX TKanX self-assigned this Feb 13, 2026
Copilot AI review requested due to automatic review settings February 13, 2026 05:16
@TKanX TKanX added the enhancement ✨ New feature or request label Feb 13, 2026
@TKanX TKanX linked an issue Feb 13, 2026 that may be closed by this pull request
12 tasks
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the topology terminology and fixes planar-center handling by generating all three DREIDING inversion terms per trigonal planar center, rather than collapsing them into a single term.

Changes:

  • Renamed topology terms across the crate: ProperDihedralTorsion, ImproperDihedralInversion, including public exports and docs.
  • Updated inversion canonicalization to keep the axis neighbor distinct while only sorting the two plane atoms.
  • Refactored the builder to emit three inversion terms per planar degree-three center and updated unit/docs expectations accordingly.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/lib.rs Updates public re-exports and Quickstart example to use torsions.
src/core/topology.rs Introduces Torsion/Inversion types + updated MolecularTopology fields and tests.
src/builder/mod.rs Renames builder helpers and changes inversion generation to emit 3 terms per planar center.
docs/ARCHITECTURE.md Updates pipeline diagram terminology to torsions/inversions.
docs/04_topology_builder.md Updates builder phase documentation and explains 3-term inversion generation.
docs/01_pipeline.md Updates topology structure terminology to torsions/inversions.
README.md Updates feature/pipeline description to include inversions and new naming.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@TKanX TKanX merged commit 12ccdbb into main Feb 13, 2026
8 checks passed
@TKanX TKanX deleted the bugfix/21-inversion-terms-collapse-to-single-instance-per-planar-center branch February 13, 2026 05:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement ✨ New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Inversion Terms Collapse to Single Instance Per Planar Center

1 participant