openforcefield / openff-toolkit

The Open Forcefield Toolkit provides implementations of the SMIRNOFF format, parameterization engine, and other tools. Documentation available at http://open-forcefield-toolkit.readthedocs.io
http://openforcefield.org
MIT License
318 stars 92 forks source link

Explicitly support well-defined periodic box vectors #1645

Open mattwthompson opened 1 year ago

mattwthompson commented 1 year ago

I can add checks similar to what OpenMM lays out here if desired

I think that would be a very reasonable invariant to establish for the entire OpenFF stack (CC @j-wags). I don't think it would inconvenience many users as many MD engines require them already, and it would greatly simplify any direct processing of box vectors (as in #707, for example). Maybe it would be problematic for loading PDB files with crystal records that are from actual crystals? We could probably write a function to automatically put box vectors in this format rather than just rejecting them if they're not? That would involve rotating coordinates themselves in the general case though.

Originally posted by @Yoshanuikabundi in https://github.com/openforcefield/openff-interchange/issues/731#issuecomment-1592266892

The toolkit has some checks on the object that's passed to Topology.box_vectors but nothing to verify the vectors make much sense. There would be value in centralizing the infrastructure on a standard, even if it's directly copied from OpenMM. If I understand the details correctly, this includes support for all sane simulation boxes I've encountered in my time. Interchange will need this validator one way or another.

mattwthompson commented 1 year ago

@Yoshanuikabundi wrote a function that I think handles this discretely: https://github.com/openforcefield/openff-interchange/blob/cf71ea548a2cf9df91ba7ad6cf2b4d287a7a3a53/openff/interchange/components/_packmol.py#L167-L190