Charmm parameters writer #848
Conversation
| Parmed stores rmin/2 in "rmin" | ||
| """ | ||
| unit_system = LAMMPS_UnitSystems("real") # ang, kcal/mol, amu |
There was a problem hiding this comment.
Do we need to make sure we're using this everywhere we write out parameters? I see we use it for the atom mass, but not anywhere else.
There was a problem hiding this comment.
Yep, for in order to handle any input unyts this will do the filtering. I was just too lazy to do it every time at first. Was wondering if you think we should make a system that isn't specific to LAMMPS for this one. It is the exact same as the real units, but maybe a touch confusing to copy them across to other engines. What do you think?
There was a problem hiding this comment.
I wonder if unit_system should be a parameter of write_prm that defaults to None. If nothing is passed, then all units are used as-is from the toplogy? If we set unit_system = topology.unit_system as default behavior, would that work as well?
def write_prm(topology, filename, unit_system=None):
if not unit_system:
unit_system = topology.unit_system
There was a problem hiding this comment.
The PRM file looks to me like it has default assumed units of kCal/mol, angstrom, amu, radians, etc for it's units. So it can only convert to one defined system.
gmso/formats/prm_writer.py
Outdated
| ) | ||
|
|
||
| f.write("\nNONBONDED\n") | ||
| nonbonded14 = topology.scaling_factors[0][2] |
There was a problem hiding this comment.
This doesn't need to be fixed here, but I feel like this attribute should be more readable rather than just knowing that 0th index is LJ and 1st index is electrostatics. Maybe a dict? I can open a separate issue.
There was a problem hiding this comment.
Was thinking the same thing myself. I think it should also be {"electrostatics":{"1-4":0.5}}, instead of a fixed list. Could potentially be some odd 1-5 interactions at some point.
There was a problem hiding this comment.
Yeah, that's sort of what I was thinking as well.
|
Cool. Thanks @chrisjonesBSU, yeah I'll punch those in this week to get this writer off the ground. I'm not sure it gets a lot of use, but it'll be nice to have. |
for more information, see https://pre-commit.ci
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| # if key not in uniqueTypesDict: | ||
| # sigma | ||
| # epsilon | ||
| # name | ||
| # mass |
Check notice
Code scanning / CodeQL
Commented-out code Note
| # else: | ||
| # vals = uniqueTypesDict[key] |
Check notice
Code scanning / CodeQL
Commented-out code Note
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #848 +/- ##
==========================================
- Coverage 94.07% 89.92% -4.16%
==========================================
Files 65 66 +1
Lines 6953 7054 +101
==========================================
- Hits 6541 6343 -198
- Misses 412 711 +299 ☔ View full report in Codecov by Sentry. |
Port over the old CHARMM/NAMD .prm file writer from mBuild to GMSO. Adding to the savers and allowing for basic charmm style potentials to be written only.
TODOS: