[WIP] Refactor MSM

[rmcgibbo]

No description provided.

[kyleabeauchamp]

Am I supposed to be reviewing this yet?

[rmcgibbo]

Not yet. I'll let you know when I'm ready.

On Wed, Jul 23, 2014 at 1:35 PM, kyleabeauchamp notifications@github.com
wrote:

Am I supposed to be reviewing this yet?

—
Reply to this email directly or view it on GitHub
#222 (comment).

[mpharrigan]

Why not use reversible_type.lower() and use 'none' instead of None to simplify case-insensitivity

[rmcgibbo]

Sure. I'll use str(self.reversible_type).lower(), which should cover it.

[rmcgibbo]

Okay. Any review would be helpful at this point.

[rmcgibbo]

The major changes are

• All matrices are dense
• Explicitly support input data that is not integers in (0, ..., n_states). Many of the tests now use input data that are strings, for example (e.g. msm.fit([['a', 'b', 'a', 'a', ...]]). This means never assuming that n_states == np.max(sequences)+1

Also, I added an option to use more aggressive ergodic trimming, with a threshold higher than just a single count in both direction. This has come up at Pande group meeting.

[mpharrigan]

Issues with the docs:

• default n_timescales is n_states - 3, this was from the sparse implementation
• countsmat_ says it returns the symmetrized counts, whereas I think it returns raw counts

Thought:

• Maybe don't have ergodic_trimming (bool) and trim_weight as two different parameters. Setting trim_weight to 0 would turn off ergodic trimming

[rmcgibbo]

Maybe don't have ergodic_trimming (bool) and trim_weight as two different parameters. Setting trim_weight to 0 would turn off ergodic trimming

+1. This is a good idea, I think.

Thanks for the other comments too.

[rmcgibbo]

@kyleabeauchamp: What do you think of the API for transform I implemented? https://github.com/rmcgibbo/mixtape/pull/222/files#diff-b4b9f1ef5ddb392515f54dc4b10fb560R183

[rmcgibbo]

I rebased onto master.

[kyleabeauchamp]

Nice.
On Jul 29, 2014 2:56 PM, "Robert McGibbon" notifications@github.com wrote:

Merged #222 #222.

—
Reply to this email directly or view it on GitHub
#222 (comment).

[rmcgibbo]

There is one bug in the tests, on py3, which is from a numpy py3 regression (numpy/numpy#641), but this shouldn't effect practical use (only present when the sequences argument to fit contains sequences with mixed types, such as [1, 1, None, 'hello', 0, 1, ...]