mosdef-hub / mbuild

A hierarchical, component based molecule builder
https://mbuild.mosdef.org
Other
171 stars 80 forks source link

HOOMD4 release #1126

Closed daico007 closed 1 year ago

daico007 commented 1 year ago

HOOMD4 is just released in the last few days and is causing our test to fail. This is due to a version check in the hoomd writer, but I tried locally and found out that our current converter can handle HOOMD4 just fine, so probably just need to relax/remove that check.

chrisjonesBSU commented 1 year ago

What kind of system were you testing hoomd4 with? One breaking change I can think of is hoomd.md.dihedral.Harmonic which is removed from version 4. We use that in hoomd_forcefield.py

405     # Set the hoomd parameters
406     # These are periodic torsions
407     periodic_torsion = hoomd.md.dihedral.Harmonic()
408     for name, dihedral_type in dihedral_type_params.items():
409         periodic_torsion.params[name] = dict(
410             k=2 * dihedral_type.phi_k / ref_energy,
411             d=1,
412             n=dihedral_type.per,
413             phi0=np.deg2rad(dihedral_type.phase),
414         )
415 
416     return periodic_torsion

This might only require a single conditional that checks the version, and creates the correct instance of the dihedral force.

daico007 commented 1 year ago

I were using HOOMD 4 for some ethanolAA for the reproducibility project. To clarify, I think I were using the pre-release 4.0 where the HarmonicDihedral has not yet been deprecated. Also, agree with your assessment of another conditional check (unless we want to stop support for the lower version completely and pin HOOMD > 3.xx (where periodic dihedral is first added))

chrisiacovella commented 1 year ago

Well, I don't think we want to stop supporting hoomd 3 since I'm sure it will be a while until people migrate versions. Allowing a user to specify 3 or 4 to get the right torsion name seems reasonable. Might be good to have a warning raised for v3 noting this change.

Note I don't think this should be something that is automatically detected, expect maybe in the CI tests. If I'm running this on my local machine, I might just be generating files to upload to a cluster which may be running a different version.

chrisjonesBSU commented 1 year ago

I agree, lets definitely keep supporting hoomd 3 as much as we can. Most of the changes in version 4 won't affect what we do in mBuild and GMSO (writing out topology to snapshot/GSD and creating the force objects).

@chrisiacovella are you suggesting something like a parameter in create_hoomd_forcefield where the user specifies which major version of hoomd they want to be used?

In your example, wouldn't the user still have to have consistency between the versions between their local machine and cluster? If they want to create hoomd3 compatible forces to use on a cluster, then they'd have to have hoomd3 installed when running on their local machine to avoid errors and vice-versa.

If I have hoomd4 installed locally, but have hoomd3 on my cluster, so I tell create_hoomd_forcefield to make a hoomd.md.Dihedral.Harmonic() torsion I'll get an error.

chrisiacovella commented 1 year ago

Oh yeah. I was thinking about it more from the context from the GSD file, but I was forgetting that only the quartet of particles would be written, not the actual force definition, so the GSD file shouldn't care if it is for v3 or v4. So, yeah, I agree it makes sense to just look at the version of the hoomd module you imported when it comes to the actual creation of the forcefield for run time. Being able to toggle versions probably wouldn't add any benefit given it would likely just break things if you tried.