-
Notifications
You must be signed in to change notification settings - Fork 830
Performance: significant speedup and memory usage improvement for PCA().run() #5352
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -238,6 +238,11 @@ class PCA(AnalysisBase): | |
| manually iterate through ``self._trajectory``, which would | ||
| incorrectly handle cases where the ``frame`` argument | ||
| was passed. | ||
| .. versionchanged:: 2.11.0 | ||
| Covariance matrix is now computed via BLAS matrix multiply instead of | ||
| per-frame outer product accumulation, and uses symmetric | ||
| eigendecomposition (``eigh``) instead of general ``eig``, resulting | ||
| in significant performance improvements for large systems. | ||
| """ | ||
|
|
||
| _analysis_algorithm_is_parallelizable = False | ||
|
|
@@ -290,6 +295,7 @@ def _prepare(self): | |
| ) | ||
| n_dim = self._n_atoms * 3 | ||
| self.cov = np.zeros((n_dim, n_dim)) | ||
| self._frame_data = np.empty((self.n_frames, n_dim)) | ||
| self._ref_atom_positions = self._reference.positions | ||
| self._ref_cog = self._reference.center_of_geometry() | ||
| self._ref_atom_positions -= self._ref_cog | ||
|
|
@@ -329,14 +335,18 @@ def _single_frame(self): | |
| else: | ||
| x = self._atoms.positions.ravel() | ||
| x -= self._xmean | ||
| self.cov += np.dot(x[:, np.newaxis], x[:, np.newaxis].T) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One reason for building the covariance matrix incrementally is that it ensures bounds on memory usage. In your approach, memory consumption grows linearly with trajectory length because you're copying coordinates into memory and thus a longer trajectory will eventually fill up the memory. The test trajectory is very short. Real trajectories are easily 1000-100,000 times longer. If there's really an advantage to multiplying huge matrices then perhaps a batched approach is a good middle ground.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Some manual benchmarking: import numpy as np
import MDAnalysis as mda; from MDAnalysis.tests import datafiles as data
u = mda.Universe(data.PSF, data.DCD)
x = u.atoms.positions.ravel()shows that replacing the >>> %timeit np.dot(x[:, np.newaxis], x[:, np.newaxis].T)
61.4 ms ± 466 μs per loop (mean ± std. dev. of 7 runs, 10 loops each)
>>> %timeit np.outer(x, x)
35.4 ms ± 249 μs per loop (mean ± std. dev. of 7 runs, 10 loops each)However, the addition update is actually slow %%timeit cov = np.zeros((len(x), len(x))); xx = np.outer(x,x)
...: cov += xx
...:
97.5 ms ± 3.08 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)so it makes sense to do the addition as part of matrix multiplication. Batching is probably the way to go (but that makes streaming more difficult). |
||
| self._frame_data[self._frame_index] = x | ||
|
|
||
| def _conclude(self): | ||
| self.cov = self._frame_data.T @ self._frame_data | ||
| self.cov /= self.n_frames - 1 | ||
| e_vals, e_vects = np.linalg.eig(self.cov) | ||
| sort_idx = np.argsort(e_vals)[::-1] | ||
| self._variance = e_vals[sort_idx] | ||
| self._p_components = e_vects[:, sort_idx] | ||
| del self._frame_data | ||
| # covariance matrix is symmetric and real-valued by | ||
| # construction, so we can use faster eigh | ||
| eigenvals, eigenvects = np.linalg.eigh(self.cov) | ||
| # eigh returns ascending order; reverse for descending | ||
| self._variance = eigenvals[::-1] | ||
| self._p_components = eigenvects[:, ::-1] | ||
| self._calculated = True | ||
| self.n_components = self._n_components | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically we can remove this, but I wasn't sure if anyone out there would expect it to be accessible from
_prepareonwards. No strong feelings either way, it's relatively cheap to initialize and maximizes backwards compatibility , but future readers might be confused by it.