markovmodel / variational

Basis sets, estimators and solvers for the variational approach of conformation dynamics. NOTE: the code has been merged with PyEMMA and is maintained there.
10 stars 5 forks source link

improved benchmarking #23

Closed franknoe closed 8 years ago

franknoe commented 8 years ago

This PR improves efficiency by correcting some mistakes in the switch for sparse treatment.

@marscher and @fabian-paul : please run python variational/estimators/tests/benchmark_moments.py again on Linux and Windows and share the resulting output text. All "speed-up" entries should be around 1 or greater.

If successful, we can start using the covariance calculation in TICA. The way to do this is as follows:

fabian-paul commented 8 years ago

Windows results: https://gist.github.com/fabian-paul/755a2f2a5780f41b96d5

franknoe commented 8 years ago

Great. No red flags.

Am 25/11/15 um 11:30 schrieb fabian-paul:

Windows results: https://gist.github.com/fabian-paul/755a2f2a5780f41b96d5

— Reply to this email directly or view it on GitHub https://github.com/markovmodel/variational/pull/23#issuecomment-159564780.


Prof. Dr. Frank Noe Head of Computational Molecular Biology group Freie Universitaet Berlin

Phone: (+49) (0)30 838 75354 Web: research.franknoe.de

Mail: Arnimallee 6, 14195 Berlin, Germany

marscher commented 8 years ago

On allegro there are also no more slowdowns https://gist.github.com/marscher/7c0f88f44548a8abb779

marscher commented 8 years ago

nsave parameter has to be at least 2, otherwise one will get an index error while adding chunks:


  File "/home/marscher/workspace/pyemma/pyemma/coordinates/transform/pca.py", line 179, in _param_add_data
    self._covar.add(X)
  File "/home/marscher/workspace/pyemma/pyemma/coordinates/estimators/covar/running_moments.py", line 219, in add
    self.storage_XX.store(Moments(w, s_X, s_X, C_XX))
  File "/home/marscher/workspace/pyemma/pyemma/coordinates/estimators/covar/running_moments.py", line 130, in store
    self.storage[-1].combine(moments, mean_free=self.remove_mean)
IndexError: list index out of range
franknoe commented 8 years ago

ah, this is a bug. Please make an issue and assign it to me.

Am 25/11/15 um 15:56 schrieb Martin K. Scherer:

nsave parameter has to be at least 2, otherwise one will get an index error while adding chunks:

File"/home/marscher/workspace/pyemma/pyemma/coordinates/transform/pca.py", line179,in _param_add_data self._covar.add(X) File"/home/marscher/workspace/pyemma/pyemma/coordinates/estimators/covar/running_moments.py", line219,in add self.storage_XX.store(Moments(w, s_X, s_X, C_XX)) File"/home/marscher/workspace/pyemma/pyemma/coordinates/estimators/covar/running_moments.py", line130,in store self.storage[-1].combine(moments,mean_free=self.remove_mean) IndexError:list index out ofrange

— Reply to this email directly or view it on GitHub https://github.com/markovmodel/variational/pull/23#issuecomment-159632080.


Prof. Dr. Frank Noe Head of Computational Molecular Biology group Freie Universitaet Berlin

Phone: (+49) (0)30 838 75354 Web: research.franknoe.de

Mail: Arnimallee 6, 14195 Berlin, Germany

marscher commented 8 years ago

I've already started to integrate the code into PCA and TICA. I'll copy the bugfixes over as soon as it is merged here.

marscher commented 8 years ago

Is it possible to provide the means? We introduced this feature for PCA/TICA but it seems this is now obsolete.

franknoe commented 8 years ago

Not yet, but easy to do. Create an issue.

Am 25/11/15 um 16:03 schrieb Martin K. Scherer:

Is it possible to provide the means? We introduced this feature for PCA/TICA but it seems this is now obsolete.

— Reply to this email directly or view it on GitHub https://github.com/markovmodel/variational/pull/23#issuecomment-159634262.


Prof. Dr. Frank Noe Head of Computational Molecular Biology group Freie Universitaet Berlin

Phone: (+49) (0)30 838 75354 Web: research.franknoe.de

Mail: Arnimallee 6, 14195 Berlin, Germany