mosdef-hub / gmso

Flexible storage of chemical topology for molecular simulation
https://gmso.mosdef.org
MIT License
52 stars 33 forks source link

Do we want the connection type type dictionary in Forcefield to have member types as keys #374

Open uppittu11 opened 4 years ago

uppittu11 commented 4 years ago

https://github.com/mosdef-hub/gmso/blob/5781bf8a4917e83cac541a0c5d9343a4d2cbc1f5/gmso/utils/ff_utils.py#L239

Currently connection types can be referenced from the forcefield using a string of the member types as the keys, like so forcefield.angle_types["opls_116~opls_117~opls_117"]. This makes it impossible to layer multiple connection types for a particular connection since the dictionary keys for these layers would clash.

A couple solutions:

rsdefever commented 4 years ago

I think I lean towards (2) or (3) -- I'd rather not put any additional burden of the FF XML creator. (3) seems cleaner to me but I am open to being convinced otherwise.

mattwthompson commented 4 years ago

Force users to lookup connections based on connections' names

I think this could be done independently of the problem of looking up layered potentials, i.e. Forcefield.angles['my_layered_angle_type1']. Name lookups are pretty slick, but AFAICT the community does not have any history of giving bonds/angles/etc meaningful names. Fortunately, they don't really have to be meaningful, just unique. For reference, here's how it's done in OpenFF, the 1.0 force field has a hundred or so bonds with names like b1, b2, etc.

>>> from openforcefield.typing.engines.smirnoff import ForceField
>>> ff = ForceField('openff-1.0.0.offxml')
>>> bond_handler.get_parameter({'id': 'b1'})
[<BondType with smirks: [#6X4:1]-[#6X4:2]  length: 1.520375903275 A  k: 531.137373861 kcal/(A**2 mol)  id: b1  >]
>>> bond_handler = ff._parameter_handlers['Bonds']

I wonder if this sort of behavior could be implemented here when a key without ~ is passed (and also falling back to an exception when no angles have names)

Of the other two, I think my preference would be for the last one, although I'll admit it's a little funky to have a lookup sometimes return a core data structure and sometimes return a list/set-like.