Update mpcd initialization for v4

[matthewddo]

Description

Update snapshot and system data to new API, currently work in progress.

To be more consistent with the new initialization API, the MPCD snapshot data structure is integrated into the HOOMD snapshot data so that MPCD particle data is synced with the HOOMD system. We are going to move the MPCD SystemData into the HOOMD SystemDefinition so that the the core data structures are also more closely synced during the simulation. This will simplify processes like initializing, replicating, etc.

Motivation and context

It didn't work anymore. :)

This is the first step in #775. Additional PRs will follow to migrate other features. These PRs will all be targeted onto mpcd-v4 as a staging branch, which is based off v4.0.0. This branch can eventually be merged into trunk-minor in a feature release because v3 did not have MPCD as part of its API.

How has this been tested?

Unit tests are being ported to pytest as features are moved.

Change log

MPCD particle data is re-enabled and included in snapshots.

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.

[joaander]

Please maintain equal performance for non-MPCD users and be aware of the get_snapshot performance improvements I made in #1538.

Write as little or as detailed information in the change log and migration guide as you prefer.

[mphoward]

Please maintain equal performance for non-MPCD users and be aware of the get_snapshot performance improvements I made in #1538.

Sounds good! We will check in #1538 and apply to MPCD since it was copied originally.

For non-MPCD users, I think these changes will fully preserve performance since there is very little overhead to the empty MPCD data structures, similarly to the topology data structures. We can be even more aggressive than we currently are, though, and not allocate any MPCD data if there are no particles present in the snapshot.

For MPCD users, I anticipate get_snapshot is going to get very slow if the user only wants access to the MD data, since it will pull all the MPCD data if present. This would not be desirable if someone wanted to do something with only the MD particles (common use case) in a snapshot. A related issue is possible in set_snapshot, where partial initialization may be desirable (e.g., only initialize from part of a snapshot). I propose adding boolean flags to the principle groups of data initialized with the following defaults:

state.get_snapshot(particles=True, topology=True, mpcd=None)
state.set_snapshot(particles=True, topology=True, mpcd=None)

(None converting to True in MPCD builds and False in non-MPCD builds). This would preserve the current defaults. I can open a separate issue after this PR is ready so we can discuss this separately.

FYI: this is still a WIP, but we are getting closer.

[cslominski]

The flags BUILD_MPCD and BUILD_VALIDATION (which appears to only be used in mpcd) are undocumented either by HOOMD or by the command cmake -LAH. I am turning off BUILD_MPCD and am not specifying BUILD_VALIDATION in my builds. Are these the correct settings, and could this information please be added to the documentation moving forward? Thanks.

[joaander]

@cslominski BUILD_VALIDATION will be removed and BUILD_MPCD will be documented by this pull request or a future pull request in this series.

[mphoward]

pre-commit.ci autofix

[mphoward]

Just a few small things to cleanup and then this is ready for review.

I accidentally merged trunk-major in when I meant trunk-minor. I think this did not add anything, and I still just need to merge in trunk-minor and fix small merge conflicts.

[mphoward]

@cslominski BUILD_VALIDATION will be removed and BUILD_MPCD will be documented by this pull request or a future pull request in this series.

BUILD_VALIDATION is now removed. I will make a note to redocument BUILD_MPCD, since it will now affect compilation.

[joaander]

I accidentally merged trunk-major in when I meant trunk-minor. I think this did not add anything, and I still just need to merge in trunk-minor and fix small merge conflicts.

Correct. trunk-major is currently behind trunk-minor (I synchronize these on releases).

[tommy-waltmann]

Great work so far on the C++ code, it looks like all the necessary changes have been made there!

Now, the python side of the code needs to be updated. Python classes need to be reorganized to use the new data structures for exposing properties in python. The names of the classes should also be capitalized and base classes should not have underscores before the class names.

After that, some basic tests can be written which make sure the python classes are functioning properly. I think some of the current C++ tests can be replaced by equivalent python tests, but that is an optional detail for later.

[mphoward]

All suggestions have been applied! For your other comments:

Now, the python side of the code needs to be updated. Python classes need to be reorganized to use the new data structures for exposing properties in python. The names of the classes should also be capitalized and base classes should not have underscores before the class names.

Yes, this will be done in many additional pull requests. This PR only implements the particle data / initialization interfaces, as the rest of the code cannot be tested without this change. The next PR should be the Integrator.

After that, some basic tests can be written which make sure the python classes are functioning properly.

Yes, there is an entire suite of Python tests that was removed during the v3 port and needs to be copied over & updated:

https://github.com/glotzerlab/hoomd-blue/tree/v2.9.7/hoomd/mpcd/test-py

For example, test_snapshot.py is a reimplementation of data_snapshot_test.py.

I think some of the current C++ tests can be replaced by equivalent python tests, but that is an optional detail for later.

I do not plan to replace the C++ tests because (1) they are already written and (2) test some things that are critical for the algorithm to work correctly but are not (and should not be) exposed at the Python level.

[tommy-waltmann]

Sounds good! FWIW, I think the rest of the python stuff can be done in a single PR and doesn't need to be split up into a long series.

[mphoward]

Sounds good! FWIW, I think the rest of the python stuff can be done in a single PR and doesn't need to be split up into a long series.

@joaander and I had discussed this a while ago, and we both think doing this in a few PRs will be easier to review. The MPCD python code & tests are probably a few thousand lines. Some of the MPCD objects also don't derive from any of the existing HOOMD classes, so new code will be needed there. I am of course planning to logically group changes into PRs when possible!

[tommy-waltmann]

Sounds good! FWIW, I think the rest of the python stuff can be done in a single PR and doesn't need to be split up into a long series.

@joaander and I had discussed this a while ago, and we both think doing this in a few PRs will be easier to review. The MPCD python code & tests are probably a few thousand lines. Some of the MPCD objects also don't derive from any of the existing HOOMD classes, so new code will be needed there. I am of course planning to logically group changes into PRs when possible!

Ok, whatever works.

[mphoward]

There is still one open review request from @klywang, but I think this PR is otherwise ready to merge!

Does somebody need to manually trigger the validation tests? Maybe I can do that, but I'm not sure how.

[joaander]

Thanks! I have one change request.

[joaander]

Thanks!