Lees-Edwards boundary conditions for shear flow

[wkdarko]

Description

This PR implements the Lees-Edwards boundary conditions for performing shear flow simulations.

Motivation and context

The implementation will allow users to simulate bulk rheology under full periodic boundary conditions.

How has this been tested?

Unit tests on box deformation methods were added.

Checklist:

• I have reviewed the Contributor Guidelines.
• I agree with the terms of the HOOMD-blue Contributor Agreement.
• My name is on the list of contributors (sphinx-doc/credits.rst) in the pull request source branch.
• I have summarized these changes in CHANGELOG.rst following the established format.

[github-actions]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[joaander]

Now that 5.4 is out, this is next on my list to review. I'll look at the proposed changes soon.

[mphoward]

Thanks! I’ve also asked Kwabena to merge up trunk-minor to keep this current.

[mphoward]

Thanks @wkdarko! This looks good to me based on our internal rounds of review. I had a couple small requests below related to the fixes you pushed to the communicators, then I think it should be good to go from my side.

[joaander]

Thanks! The code is clean and the changes are minimal.

I am concerned about the large number of minImage, wrap, and shift methods. Some return results, others modify references in place. To someone reading this code a year from now, it will not be clear what method is called. Could you add a suffix to the new methods to ameliorate this problem?

This PR is also missing a user interface to set the deformation rate and documentation of the feature in features.rst. We need to clearly document what classes in HOOMD-blue interoperate with the deformation rate and what ones do not. At this point, it is not clear to me whether that can be done in features.rst or if it needs to be mentioned in each class's documentation. Ideally, classes that don't support box deformations will raise an error when the deformation rate is non-zero.

Longer term (after this PR), you should add a tutorial to https://github.com/glotzerlab/hoomd-examples that shows users how to perform Lees-Edwards simulations.

[mphoward]

I am concerned about the large number of minImage, wrap, and shift methods. Some return results, others modify references in place. To someone reading this code a year from now, it will not be clear what method is called. Could you add a suffix to the new methods to ameliorate this problem?

The methods we added have the same argument / return types as the existing methods, so we thought overloading to include a velocity argument would be cleaner than appending a suffix. Would it be OK if we address this by expanding the doxygen documentation for the new methods? We can explain that they are used in the same way as the position-only methods but when the velocity also needs to be manipulated. Otherwise, we will need to go back through all the files in the diff manually to rename.

This PR is also missing a user interface to set the deformation rate and documentation of the feature in features.rst. We need to clearly document what classes in HOOMD-blue interoperate with the deformation rate and what ones do not. At this point, it is not clear to me whether that can be done in features.rst or if it needs to be mentioned in each class's documentation. Ideally, classes that don't support box deformations will raise an error when the deformation rate is non-zero.

Yes, we are working on the user interface to do this in a separate PR! We were worried this one was getting too bulky. If you are OK with it, I think this PR could be merged as a temporarily undocumented feature because it doesn't change any behavior of existing code without an interface to set the deformation rates. (They are all zero by default.)

@wkdarko left a comment on #2120 to ask about how it would be best to implement the deformation, and we would appreciate your feedback there before we proceed too far!

Because of how we have modified the BoxDim, all the MD integration methods and pair forces should work with the deformation. I'm not sure if there are any other MD features that would break... the thermo compute will return unphysical values, but that is expected for NEMD. I know that some things in MPCD will break (and we will fix them), so we will error out and document that in the follow up PR until we have them fixed. We plan to do some thorough validation and comparison with LAMMPS once we have the user interface created.

Longer term (after this PR), you should add a tutorial to https://github.com/glotzerlab/hoomd-examples that shows users how to perform Lees-Edwards simulations.

Yes, definitely will do!

[joaander]

@mphoward minImage is the method I was referring to that returns a value in two out of four overloads, but not in the other two. I understand that C++ lacks a natural syntax for multiple return value unpacking, so I don't have a better suggestion.

[mphoward]

Oh, I see what you mean. If you have a strong preference toward something like minImageWithVelocity(dx, dv) we can do that.

[joaander]

No, I don't have a strong preference. Perhaps it is best to leave as is and expand the documentation as you suggest.

[mphoward]

I'll work with @wkdarko to expand this documentation to see if we can resolve it that way.

[joaander]

Yes, we are working on the user interface to do this in a separate PR! We were worried this one was getting too bulky. If you are OK with it, I think this PR could be merged as a temporarily undocumented feature because it doesn't change any behavior of existing code without an interface to set the deformation rates. (They are all zero by default.)

Yes, I would prefer everything in one PR. Reviewing the changes in batches is helpful.

@wkdarko left a comment on #2120 to ask about how it would be best to implement the deformation, and we would appreciate your feedback there before we proceed too far!

Understood, I will move this discussion there.