search

markovmodel / msmtools

Tools for estimating and analyzing Markov state models
GNU Lesser General Public License v3.0
40 stars 26 forks source link

[effective_counts|statistitcal_inefficiency] fix performance due to c… #64

Closed marscher closed 8 years ago

marscher commented 8 years ago

…alling setter of csr matrix.

Partially fixes #34

for a dtraj list like this: 100 states, len=10000, number=10 the runtime drops from 55s to 37s.

franknoe commented 8 years ago

Thanks! The new function should give identical results to the old function - have you tested this?

Why aren't the integration tests being started?

marscher commented 8 years ago

Yes I've compared Ceff with allclose, btw. all of the stat inefficiency stuff is pretty much untested. Travis has finished, hasn't it?

franknoe commented 8 years ago

Am 18/11/15 um 15:49 schrieb Martin K. Scherer:

Yes I've compared Ceff with allclose, btw. all of the stat inefficiency stuff is pretty much untested.

Yes, it's hard to come up with a test case.

Travis has finished, hasn't it?

Now it has - when I had checked, I didn't even get a test report at all. Weird.

— Reply to this email directly or view it on GitHub https://github.com/markovmodel/msmtools/pull/64#issuecomment-157737503.

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

Github introduced a new security setting for third-party application authorization upon organisations, had to disable that.

marscher commented 8 years ago

Actually there is a test case which failed on Appveyor for a different shape of si.

franknoe commented 8 years ago

Can you please add a test which uses the effective count matrix from the old impl e.g. with the double-well microstate dtraj in our PyEMMA data set as a reference result? That way we'll at least be safe to algorithmic errors until we have more systematic tests.

Am 18/11/15 um 15:49 schrieb Martin K. Scherer:

Yes I've compared Ceff with allclose, btw. all of the stat inefficiency stuff is pretty much untested. Travis has finished, hasn't it?

— Reply to this email directly or view it on GitHub https://github.com/markovmodel/msmtools/pull/64#issuecomment-157737503.

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

Ok, will add this test for now.

marscher commented 8 years ago

This test would require copying the old code over to the unittest (unfiddeling all the dependencies this code has). Moreover the test data set is not yet available in msmtools - do you want me to copy this?

marscher commented 8 years ago

I'll add the matrix generated with the old code to the test files.

franknoe commented 8 years ago

Yes, that's what I mean

Am 18/11/15 um 16:05 schrieb Martin K. Scherer:

I'll add the matrix generated with the old code to the test files.

— Reply to this email directly or view it on GitHub https://github.com/markovmodel/msmtools/pull/64#issuecomment-157741674.

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