mphowardlab / azplugins

A HOOMD-blue component for soft matter simulations.
BSD 3-Clause "New" or "Revised" License
21 stars 13 forks source link

Feature/velocity profile #26

Closed astatt closed 4 years ago

astatt commented 4 years ago

Simple helper class to determine 1D velocity profiles, e.g. v_z(x) during the simulation. Future improvements could/should include:

  1. check for box size fluctuations for pure md simulations (mpcd systems error out anyway), parameter checks (will try to do that soon)
  2. check if the path for the filename exists/ is writable
  3. allow measuring multiple velocity components along the same spatial direction with one instance of the class, e.g. measure v_x(x), v_y(x),..
  4. measure temperature profiles
  5. 2D profiles (probably separate class)
mphoward commented 4 years ago

Do you want me to have a look at this now, or are you still working on some parts of it?

mphoward commented 4 years ago

I dug into some of my files, and I made some changes to the API. I think this signature simplifies some of the usage. In terms of the I/O, I think it should be left up to the user to decide how to do the averaging, since this is a little complicated. This object just does the running average, and then it could be wrapped by another type of analyzer that decides if writing should be done periodically or only at the end.

Should we add a reset method that zeros the internal counters?

This also needs testing--I'm sure there are typos in it. :-) Let me know what you think!

astatt commented 4 years ago

I'll do some user-level testing and carefully reading on the weekend, will not have time before that. The API is much simpler than what I had, which is probably good. A reset call would be nice, gives the user more flexibility in how they want to use it.

If we ever want to implement a 1D temperature (for evaporation) or a 2D velocity (for channel flows) version of this, I think your API is more flexible as well.

mphoward commented 4 years ago

OK, thanks! I caught some typos and also added the reset method. Let me know when you try out the code, and feel free to add comments to the PR if anything needs to be fixed. If it's successful, we can write a small unit test to make sure it's binning and averaging correctly, and then this should be good to go.

mphoward commented 4 years ago

I had forgotten about this PR :-) Do you have any additional feedback on it, or is it ready to go?

astatt commented 4 years ago

Do you think this should have a unit test? Since we are just using numpy, a unit test might be overkill. If no unit test needed, it looks ready to be merged to me.

mphoward commented 4 years ago

We should probably test it, but we can do it a lazy way where we initialize a system with a couple particles in it with fixed velocities and then make sure we get the right values back.

astatt commented 4 years ago

I've added a very simple lazy unit test, where I just computed the expected histograms by hand and made sure it records the right values. If you think that's good enough for a unit test, this can be merged.

mphoward commented 4 years ago

Thank you! Yes, this is exactly the level of test I had in mind. It looks like there are some MPI errors, which I think are in both the base code and the unit tests. I will correct those, and then this should be good to merge.