ag-csw / LDStreamHMMLearn

1 stars 0 forks source link

Investigate performance comparison of naive and bayes methods for MM estimation #22

Closed greenTara closed 7 years ago

greenTara commented 7 years ago

I would like to change the calculation of performance time to be more inclusive, since we have been getting the rather surprising result that naïve and bayes performance are the same. So the lines for setting t0 and t1 should be moved (up and down, respectively) so that we are capturing more of the process.

I am thinking it might be more reliable if we have separate for loops for the naïve and bayes calculations, and we get the vector of times by sampling once for each pass through the loop.

greenTara commented 7 years ago

Let's start this investigation by duplicating the stationary script for the taumeta heatmaps. That way, we can run the two scripts and compare output.

greenTara commented 7 years ago

Option 1. Split performance_and_evaluation method into two methods, one for naive and one for bayes.

greenTara commented 7 years ago

Option 2. Keep one method, but have two for loops, one for naive, one for bayes

greenTara commented 7 years ago

The advantage of option 1 is that we can separate several concerns- a) comparing the expected errors of the naive and bayes methods, which requires averaging over many runs b) comparing the performance of the naive and bayes methods, which does not require multiple runs c) investigating the dependence of error and performance on model and algorithm parameters, for the bayes algorithm alone, which does not require performing the naive computation

greenTara commented 7 years ago

The only advantage of option 2 that i can think of is that it is less of a change in the code, and so is less likely to break things.

greenTara commented 7 years ago

A separate concern is in regard to what should be timed.

Currently we are leaving out: 1) the initial selection of data slices 2) extra steps involved in computing the error by taking the difference from the ground truth transition matrix.

Item 2 is legitimate to omit, since this would not be done in an actual calculation where the ground truth model is not known. However, Item 1 should be included in the performance computation.

greenTara commented 7 years ago

Also there is an initial call to estimate_via_sliding_windows - the rationale behind this step that is stated in the comments doesn't make sense to me now.

"initialization - we found out that calling the count_matrix_coo2_mult function the first time results in lower performance than for following calls - probably due to caching in the background. To avoid this deviation, we call this function once - for starting the cache procedure."

If there is some caching being done, clearly we need to include that in the performance computation, and have this be done fairly when comparing the naive and bayes methods.

greenTara commented 7 years ago

Decision: take option 1. For now, encapsulate the two separate methods into one overall method that calls both, and the same name and signature as the current method.

alexlafleur commented 7 years ago

util_evaluation_mm_old.py is the old script with naive and bayes calculations in one method, and with timing of count_matrix_generation only. util_evaluation_mm.py is the new script with separate functions for naive and bayes calculations, and adapted timing which also includes the dataslice and transition matrix generation.

greenTara commented 7 years ago

We were observing no significant difference in performance between naive and bayes in earlier trials. I believe this is because the step that has different complexity in the two - the generation of the count matrix - involves only additions, and so runs very fast, compared to the calculation of the transition matrix from the count matrix, which involves division. However, if we increase num_trajectories enough, the part of the estimation that differs between naive and bayes should become apparent. We need to run some trials - using MM because it runs faster - to see how large num_trajectories needs to be in order for the performance difference to manifest.

greenTara commented 7 years ago

Done.

Plot is archived in issue #38