Fix mass type in convert_parmed

[marjanalbooyeh]

I found the following error in a test script where a GMSO topology is first converted to a Parmed structure using to_parmed and then the same structure is converted back to GMSO topology using from_parmed.

Here's the test script:

import forcefield_utilities as ffutils
import gmso
from gmso.external import to_parmed, from_parmed, from_mbuild
from gmso.parameterization import apply
import mbuild as mb

methane_box = mb.fill_box(compound=[mb.load("C", smiles=True)], n_compounds=5, box=[2,2,2])
gmso_top = from_mbuild(methane_box)
oplsaa = ffutils.FoyerFFs().load('oplsaa').to_gmso_ff()
apply(top=gmso_top, forcefields=oplsaa, identify_connections=True)
pmd_struc = to_parmed(gmso_top)
back_to_gmso = from_parmed(pmd_struc)

error:

  File "/gmso/gmso/external/convert_parmed.py", line 61, in from_parmed
    pmd_top_atomtypes = _atom_types_from_pmd(structure)
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/gmso/gmso/external/convert_parmed.py", line 314, in _atom_types_from_pmd
    pmd_top_atomtypes[atom_type] = top_atomtype
    ~~~~~~~~~~~~~~~~~^^^^^^^^^^^
  File "/anaconda3/envs/gmso-dev/lib/python3.11/site-packages/parmed/topologyobjects.py", line 5312, in __hash__
    return hash((self.name, self._round_trunc(self.mass), self.atomic_number, self.bond_type,
                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/anaconda3/envs/gmso-dev/lib/python3.11/site-packages/parmed/topologyobjects.py", line 5319, in _round_trunc
    return round(value, _TINY_DIGITS) if value is not None else None
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: type numpy.ndarray doesn't define __round__ method

The source of error is the incorrect type of atype_mass in _atom_types_from_gmso method, which should be float rather than numpy array.

[marjanalbooyeh]

@CalCraven I noticed that in _atom_types_from_gmso, the structure parameter is not being used in the function and pmd_atom variable is created at the end but not returned. Is that expected?

[CalCraven]

These changes look good, yeah it seems the .value() function is returning a numpy array since unyt always treats it's elements as unyt_array objects. Since mass should always be a singular value, I think this should work fine.

It's also nice to know things are lossless to and from parmed. Would it be worthwhile to add a single test for a typed molecule with impropers to make sure those are handled correctly? I think there's a typed_dimethylnitroaniline molecule in base_tests that could be used.

[codecov]

Codecov Report

Patch coverage: 50.00% and project coverage change: -0.02% ⚠️

Comparison is base (1d23ffb) 91.95% compared to head (054e544) 91.94%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #754      +/-   ##
==========================================
- Coverage   91.95%   91.94%   -0.02%     
==========================================
  Files          67       67              
  Lines        6815     6814       -1     
==========================================
- Hits         6267     6265       -2     
- Misses        548      549       +1     

Files Changed	Coverage Δ
gmso/external/convert_parmed.py	95.11% <50.00%> (-0.02%)	⬇️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[chrisjonesBSU]

LGTM! Thanks @marjanAlbouye

[CalCraven]

@CalCraven I noticed that in _atom_types_from_gmso, the structure parameter is not being used in the function and pmd_atom variable is created at the end but not returned. Is that expected?

I think you're right, structure can be removed from that function. My guess is it's leftover code from something that did need it at some point.

the pmd_atom isn't made in that function though, it's the pmd.Atomtype. And then using the map, which is a map where the values are the pmd.Atoms, then each created atomtype is assigned to that atom here.

Because those objects are still in memory when the function is called, and after it's finished, essentially the attribute is saved as soon as they're assigned, and don't need to [get passed back to the top level function](_atom_types_from_gmso(top, structure, atom_map)) and assigned there.

[CalCraven]

LGTM!