search

glotzerlab / hoomd-blue

Molecular dynamics and Monte Carlo soft matter simulation on GPUs.
http://glotzerlab.engin.umich.edu/hoomd-blue
BSD 3-Clause "New" or "Revised" License
329 stars 127 forks source link

Port Table potentials to v3 API #360

Closed joaander closed 2 years ago

joaander commented 5 years ago

HOOMD's tabulated potential implementation and API are being very widely used. Their implementation, and Python API in particular, could use a thorough review and be improved if possible.

csadorf commented 5 years ago

I'm interested in contributing to this.

joaander commented 5 years ago

In connection with #398 a new table potential API might expose a property that provides a proxy interface to the tables. Tables would be stored on the C++ side and exposed to python as numpy arrays.

mphoward commented 3 years ago

When this gets revisited, I wanted to note that there is a small bug in set_from_file, where the file handle is opened without an explicit call to close. This handle is sometimes not closed correctly. Opening the file as a context should fix this.

joaander commented 3 years ago

Thanks for pointing that out. It isn't documented here but the plan will be to offer numpy array access to table data. I would expect to remove the set_from_file and functional evaluation syntactic sugar and let the user use standard python tools to read or evaluate table data in whatever form they choose (HDF5 file, text file, sympy expression, etc..)

mphoward commented 3 years ago

That's even better! We are currently wrapping hoomd in some software that will support multiple simulation packages. Our pair potentials are callable objects that we have already tabulated, so it would be great if we could just set the table values directly. We started by dumping to disk & reading back in, then we caught this warning about the file handle from a unittest, so now we're bypassing the file and wrapping the tables in interpolating functions (like what set_from_file is doing anyway). It'd be great to skip all that.

As an aside, would it also be possible to support tables with different widths per pair? Maybe by aligning the internal memory to the largest table or by allocating each table separately? It's nice that each pair can have a different rmin / rmax, but the constant table width means that you can't hold the spacing dr constant if rmin and rmax are not the same.

joaander commented 3 years ago

@mphoward #965 enables different widths per type pair in the v3 API for pair potentials. The PR is awaiting final review from @b-butler and then it will be merged.

We haven't currently started work on table potentials for bonds, angles, or dihedrals. We could implement the same features and follow the implementation in #965 but only if we complete #490 first. Given the complexity of this work and that I'd like to get a 3.0.0 release complete sooner rather than laster, we may defer this work for a future 3.x release.

mphoward commented 3 years ago

Great! This looks like it will work nicely. I agree that #490 should be done before porting the other types of potentials, but this would be a decent amount of work.

joaander commented 2 years ago

Bond, angle, and dihedral potentials are not commonly used or requested. In the interests of completing 3.0.0 soon, I am deferring their implementation to a future 3.x release.