changed TICC to a class with predict method.

[mohataher]

Moved all TICC methods to a class named TICC located inside TICC_SOlver.py. Changed method solve to method fit. Extracted several methods to make it easier to read and debug the code. These changes simplifies the code call and implementation and paves the way to add predict method. More information to be found on issue #18.

[davidhallac]

Hi! Thank you so much for these changes - they look great!

I wrote a couple unit tests to compare the old code with your modified code, and they seem very similar but not exactly the same. I just pushed 2 unit tests to the master repo. The first test yields the same results on both solvers, but the second one (test_multiExample) is slightly different...

More specifically, the old code yields the following segments (StartTime, EndTime, ClusterID):

[(0, 38, 2.0), (39, 105, 1.0), (106, 128, 2.0), (129, 333, 1.0), (334, 814, 2.0), (815, 883, 1.0), (884, 996, 2.0), (997, 1399, 0.0), (1400, 1880, 2.0), (1881, 1916, 0.0), (1917, 2125, 2.0), (2126, 2148, 0.0), (2149, 2410, 2.0), (2411, 2433, 1.0), (2434, 2952, 2.0), (2953, 3006, 0.0), (3007, 3547, 2.0), (3548, 3568, 0.0), (3569, 3739, 2.0), (3740, 3804, 1.0), (3805, 6439, 4.0), (6440, 6481, 1.0), (6482, 8431, 4.0), (8432, 9866, 1.0), (9867, 9977, 2.0), (9978, 10006, 0.0), (10007, 10245, 2.0), (10246, 11537, 0.0), (11538, 15509, 3.0), (15510, 15550, 0.0), (15551, 15672, 3.0), (15673, 15685, 0.0), (15686, 18888, 3.0), (18889, 18923, 0.0), (18924, 19109, 3.0), (19110, 19133, 0.0), (19134, 19602, 3.0)]

Your modified code returns:

[(0, 38, 2.0), (39, 106, 7.0), (107, 128, 2.0), (129, 334, 7.0), (335, 813, 2.0), (814, 883, 7.0), (884, 995, 2.0), (996, 1400, 7.0), (1401, 1703, 2.0), (1704, 1729, 7.0), (1730, 1880, 2.0), (1881, 1916, 7.0), (1917, 2124, 2.0), (2125, 2149, 7.0), (2150, 2410, 2.0), (2411, 2432, 4.0), (2433, 2952, 2.0), (2953, 3006, 7.0), (3007, 3547, 2.0), (3548, 3567, 0.0), (3568, 3736, 2.0), (3737, 3799, 7.0), (3800, 4638, 3.0), (4639, 5382, 1.0), (5383, 5433, 3.0), (5434, 6232, 1.0), (6233, 6436, 3.0), (6437, 6735, 5.0), (6736, 8432, 3.0), (8433, 8500, 4.0), (8501, 8713, 5.0), (8714, 8938, 4.0), (8939, 9133, 5.0), (9134, 9382, 4.0), (9383, 9853, 5.0), (9854, 9867, 7.0), (9868, 9977, 2.0), (9978, 10007, 7.0), (10008, 10246, 2.0), (10247, 11538, 0.0), (11539, 15510, 6.0), (15511, 15549, 0.0), (15550, 15672, 6.0), (15673, 15685, 0.0), (15686, 18889, 6.0), (18890, 18923, 0.0), (18924, 19109, 6.0), (19110, 19133, 0.0), (19134, 19602, 6.0)]

It seems like the two methods are initialized slightly differently, and this different initialization leads to slightly different results (the clusterIDs being shuffled doesn't matter as much as the different breakpoints...). It looks like the initialization's are identical but maybe I'm missing something?

[mohataher]

Based on #30 , could these test cases be failing due to the random number generation issues?

[davidhallac]

Yup, you're totally right! The difference was due to the random initialization, which has been fixed in the latest push. I have merged your changes with the newest version of TICC (where we have since added Python3 support, along with a few other small changes). Thanks so much for the help!

[mohataher]

Thank you for the effort of merging this. The algorithm is promising and I'm happy to contribute more to this.