gmso
gmso copied to clipboard
Add LayeredDihedral and LayeredImproper support
Closes #561.
Checklist
- [X] Class Definitions
- [x] Topology Changes (WIP)
- [x] Tests
- [ ] Other changes to formats/external modules (May be suited for a different PR)
CC:
- @rsdefever for cassandramcf/parmed handling @CalCraven for networkx handling @daico007 for mbuild/openmm changes
@bc118 additional comments (10-24-2023):
Multiple Dihedral types (equations) not accepted (Example: CHARMM style with Periodic and Harmonic dihedrals)
Multiple dihedral types should be accepted for both the proper and improper dihedrals. This is the standard input in the CHARMM format. However, in CHARMM, the harmonic dihedrals (proper and improper) are listed as periodic with n=0 (yes, this can be confusing.
We should allow different dihedral types, entering correctly the periodic and harmonic forms as the equations define (see the attached XML). In MoSDeF-GOMC, we will read the proper and improper harmonic dihedrals and write them as the proper and improper periodic dihedrals with n=0 for the CHARMM FF style (required for CHARMM style format - yes, it is confusing). Each writer should handle this separately for their suited engines, where this specific formatted example is for NAMD and the future GOMC once harmonic dihedrals are added.
This is critical if we ever want to be able to use the CHARMM FF in MoSDeF force fields/simulations.
I have provided an example of a proper periodic and harmonic dihedral that fails due to the different dihedral styles.
Codecov Report
Merging #569 (ca1afa1) into main (3a387c7) will decrease coverage by
0.06%
. The diff coverage is90.97%
.
@@ Coverage Diff @@
## main #569 +/- ##
==========================================
- Coverage 90.79% 90.72% -0.07%
==========================================
Files 56 56
Lines 4626 4699 +73
==========================================
+ Hits 4200 4263 +63
- Misses 426 436 +10
Impacted Files | Coverage Δ | |
---|---|---|
gmso/core/improper.py | 90.90% <87.50%> (-9.10%) |
:arrow_down: |
gmso/core/topology.py | 95.89% <89.13%> (-0.58%) |
:arrow_down: |
gmso/core/dihedral.py | 96.55% <95.34%> (-3.45%) |
:arrow_down: |
gmso/__init__.py | 100.00% <100.00%> (ø) |
|
gmso/abc/abstract_connection.py | 97.87% <100.00%> (ø) |
|
gmso/abc/gmso_base.py | 97.36% <100.00%> (+0.03%) |
:arrow_up: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 3a387c7...ca1afa1. Read the comment docs.
Perhaps something to watch out for:
Traceback (most recent call last):
File "/Users/ryandefever/Desktop/test_serialize/bmim/run.py", line 9, in <module>
top = from_parmed(pmd)
File "/Users/ryandefever/repos/mosdef/gmso/gmso/external/convert_parmed.py", line 232, in from_parmed
top.update_topology()
File "/Users/ryandefever/repos/mosdef/gmso/gmso/core/topology.py", line 947, in update_topology
self.update_connection_types()
File "/Users/ryandefever/repos/mosdef/gmso/gmso/core/topology.py", line 614, in update_connection_types
if isinstance(c.connection_type, AngleType):
AttributeError: 'LayeredDihedral' object has no attribute 'connection_type'
Perhaps something to watch out for:
Traceback (most recent call last): File "/Users/ryandefever/Desktop/test_serialize/bmim/run.py", line 9, in <module> top = from_parmed(pmd) File "/Users/ryandefever/repos/mosdef/gmso/gmso/external/convert_parmed.py", line 232, in from_parmed top.update_topology() File "/Users/ryandefever/repos/mosdef/gmso/gmso/core/topology.py", line 947, in update_topology self.update_connection_types() File "/Users/ryandefever/repos/mosdef/gmso/gmso/core/topology.py", line 614, in update_connection_types if isinstance(c.connection_type, AngleType): AttributeError: 'LayeredDihedral' object has no attribute 'connection_type'
@rsdefever Thanks for the catch. cfd9125 adds the check while updating connection types.
This pull request introduces 2 alerts when merging fa751fa3b3417ccabd54c5025f075048efbc6c1b into 00f329cfbe1decc087172ddc9421ed804c58e44b - view on LGTM.com
new alerts:
- 2 for Unused import
Can we add a test where there are dihedrals of different functional forms in the layered dihedral? The actual handling of that will be writer-dependent, but we can support that as we develop the writers.