mattjj / pyhsmm

MIT License
546 stars 173 forks source link

Bug? #19

Closed karidajiang closed 10 years ago

karidajiang commented 10 years ago

Hi,

I successfully built your code and produced results on the example data with 5000 points. While reading your code, however, I came across these lines that I do not understand...

In internals/hsmm_messages.h, function messages_backwards_log, when you are calculating the backward messages:

for(int i=0; i<M; i++) {
    if (ebetastarl(t,i) != ebetastarl(t,i)) {
        ebetastarl(i,t) = -1.0*numeric_limits<Type>::infinity();
    }
}

This piece of code appeared twice, once after you computed ebetal and another time after you computed ebetalstral. I assume you are trying to detect whether there exists NaN in your matrix, but why is it in the if statement, the indices are flipped? i.e. ebetastarl(t,i) became ebetastarl(i,t) ? When t = 4999 and you only have 25 states, will ebetastarl(i,t) lead to an out-of-range exception? (at least on Eigen::Matrix it will have an assertion error?)

Thanks!

mattjj commented 10 years ago

That's definitely a bug, and as you point out it would lead to out-of-bounds writes when there are NaNs computed in the messages arrays.

I think those lines would not typically be executed; NaNs should only occur if all the emission log likelihoods (in aBl) for a certain time index are all -inf, or if a similar 'data has zero probability' situation happens due to an interaction between some -inf log likelihoods (or underflow) and -infs in the log transition matrix. So this bug probably doesn't come up much (if at all), but it's definitely a bug.

Thanks for finding this!