write_gsd doesn't work when no mass is set

[chrisjonesBSU]

write_gsd doesn't work if masses aren't set in the topology.

129 masses = np.array([site.mass for site in top.sites])
    130 masses[masses == 0] = 1.0
--> 131 gsd_snapshot.particles.mass = masses / ref_mass

It looks like it tries to correct for this case (line 130), but the problem is that top.site.mass defaults to None if it hasn't been set.

This is also related to #664

I think a couple fixes are:

1. Set default site.mass to 0 instead of None
2. Add another line masses[masses == None] = 1.0 to correct for None masses
3. Go through our external from_* functions and try to populate site.mass correctly when the information is available
4. Check for None masses and throw an error message instructing the user to populate the site masses

[chrisjonesBSU]

It also fails due to site.charge defaulting to None see #679

[CalCraven]

Makes sense to me. I think the preferred behavior would be to check for mass == None values, since the None input gives us a bit more information than the 0. A None just means no mass has been associated with the site, and that allows some of the different automated ways to grab the mass to kick in, such as from the atomtype or fromele interpreting the element from the name. Therefore I think leaving None as the default is good. Honestly, auto setting it to 1 seems like bad practice because you could miss that the masses are not saved and then they get overwritten with that 1 value.

I almost think this issue necessitates some hierarchy of mass treatment. We could make a module which gives the many ways to check the mass for a site, and we can specify a given order for the specific writer instance.

1. from site.atomtype.mass
2. from site.mass
3. from ele.element_from_name(site.name)
4. from site_max_sigma.sigma / site.sigma * site_max_sigma.mass (the ratio of the current site's sigma to the max sigma in the system)
5. Raise Warning that can be turned off in expert mode

[chrisjonesBSU]

I agree. I'll just add an extra line that corrects for None masses and charges.

[CalCraven]

I think that's the easiest fix. Where are you setting the mass from in this instance?

[chrisjonesBSU]

I came across this issue going form mbuild --> gmso --> gsd file. Ultimatley, using it as a replacement for the gsd writer in mBuild (which uses Parmed) and doesn't populate angles and dihedrals.

And since from_mbuild doesn't carry over the mass or charge informaiton (#679 #664) these values were being left as None

[chrisjonesBSU]

I see what you mean, but are you saying that from_mbuild shouldn't carry over and populate a site's mass or charge value, and instead that should be done after the fact in gmso during atom typing and applying a FF?

[CalCraven]

I think this conversation is happening in another thread, but certainly if that information is in the mBuild object it should get converted to the GMSO Atom. That's important data. So the convert_mbuild function will need to check for that information and add it. However, if you atomtype and get the mass from an atomtype, which should be preferred. Just thinking out loud here, but I'm almost thinking mass and charge should come from the individual site. Any thoughts about that?

[daico007]

About the charge issue, I think that's almost depends on what type of forcefield you are using. Like for oplsaa the charge is tied with the atomtype, but for some others, the atomtype will define the sigma and epsilon, and the charge will be assigned to the site in a separate step. For the first case, though, I can see the scenario where we have the system typed, but then need to make a few tweaks to make it charge-neutral, in which case, I think modifying the site.charge would be better. So, I think site.charge should take precedent to atomtype.charge.

[CalCraven]

I think that makes sense also as the default. But I think a specific writer can tie the style to where they want to access the charge. So they can write from site.atomtype.charge as the first precedence. But in terms of converting from mBuild, I think we have to associate with site, as @daico007 says.

[chrisjonesBSU]

Closed by #680