proteneer / timemachine

Differentiate all the things!
Other
138 stars 17 forks source link

Reduce Memory used for Coordinates in Exchange Moves #1241

Closed badisa closed 7 months ago

badisa commented 7 months ago

Previously the coordinates for the new proposals were stored with all N atoms, which will cause difficulties (memory usage, duplicating coords repeatedly) when the number of proposals per step is greater than 1. To avoid this, this PR only stores the rotated/translated atom coords of each proposal. This allows us to more easily batch as well as removes the need to do a memcpy (~0.3% of the runtime) on coordinates.

This PR also cleans up the terminology and tries to document some of the changes that were made in previous PRs which weren't updated when they should have been.

badisa commented 7 months ago

Added determinism test in https://github.com/proteneer/timemachine/pull/1241/commits/8c37fd369f070e767f42d335ee5f0d627628e83d

Matches master exactly, so this change has no impact on the bitwise determinism (as expected).

I didn't put the data into timemachine.testsystems like some of our other data as it doesn't seem like the type of thing we want installed with the package (which is why it doesn't use importlib). We seem to currently install test data with the timemachine package, but doesn't seem useful for ship test data with the package (unless perhaps users install with [test])`