hoomd-blue
hoomd-blue copied to clipboard
glotzerlab
Add support for creation of box object from non-triangular matrices
Description
Current implementation of hoomd's box.from_matrix only supports upper triangular matrices, and the class is lacking support for creating boxes from general cell matrices. This PR should enable this. This should be a non-breaking change since upper triangular matrices should still work in the same manner. EDIT: A new method box.from_basis_vectors was added instead of changing the functionality of from_matrix. This leaves freud inconsistency unresolved.
Motivation and context
Freud's box class has a from_matrix that supports both (upper) triangular matrices as well as other general cell matrices. On previous dev-meeting we discussed that this behavior should be the same between the two codes and that hoomd's box.from_matrix should be updated to also support box matrices that are not upper triangular ones.
How has this been tested?
I have added a test which tests creation of box object from a valid cell matrix that is not an upper triangular one.
Change log
*Added*
* ``hoomd.Box.from_matrix`` - Added support for general cell matrices.
Checklist:
- [x] I have reviewed the Contributor Guidelines.
- [x] I agree with the terms of the HOOMD-blue Contributor Agreement.
- [x] My name is on the list of contributors (
sphinx-doc/credits.rst) in the pull request source branch.
It is nice to add generality, but we should to be clear in the documentation that the resulting
Boxis rotated relative to the input vectors the user provides (when they are not upper triangular).Namely, the output of
to_matrixwill not match the same as the input tofrom_matrixin general.
I'll update the docs.
Otherwise, users might expect to be able to place new particles within the box defined by their original lattice vectors. Is there a way for the users to get the resulting rotation so that they can transform their input coordinates and orientations into the new box frame?
This must be doable. How should this rotation be given back to the user? Property of the box?
This must be doable. How should this rotation be given back to the user? Property of the box?
That is unclear. Ideally, the function could return a tuple (box, rotation), but that would be an API breaking change. Perhaps an example code in the docs that works from the original matrix and the resulting output from_matrix? rowan might have functions that help here.
After discussing offline: we could name the feature from_basis_vectors. This could return the new box and the rotation matrix as a tuple (as desired), and it maintains the symmetry between from_box and to_box. It does not fix the naming mismatch between Freud and HOOMD though, so it's not a perfect solution.
I don't know what happened here, but this branch is deleting files and modifying many files that it should not. I am restoring them now.
It seems that there were changes that undid many recent pull request merges: #1758, #1757, #1756, #1748, #1772 and possibly more. I will squash merge to clean up the history.
For future reference: Merging from a trunk-patch up into a branch that will be merged onto trunk-minor is fine. All those commits will eventually be merged there. The problem is when you merge trunk-minor or trunk-major into a branch that is planned to be merged into trunk-patch or trunk-major into a branch that will be merged into trunk-minor. To fix such an accidental merge, git reset --hard <commit before merge> and git push --force.
Tests were failing on my macOS machine due to floating point errors from linalg.solve. Swapping to np.float64 instead of np.float32 fixed this, and brings the default in line with the previous from_matrix function which used double precision by default. I also parametrized some of the new tests to check a few extra values.