New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug in HMM #600

Closed
warpuv opened this Issue Apr 1, 2016 · 4 comments

Comments

Projects
None yet
2 participants
@warpuv

warpuv commented Apr 1, 2016

In file mlpack\methods\hmm\hmm_impl.hpp, line 169:

    if (dataSeq.size() == 0)
      initial = newInitial / dataSeq.size();

Should be probably dataSeq.size() > 1.
mlpack version 2.0.1

@rcurtin

This comment has been minimized.

Show comment
Hide comment
@rcurtin

rcurtin Apr 5, 2016

Member

There's something deeper that's wrong here, but I need to find my copy of the Elliot, Aggoun, and Moore book to figure out exactly what's up. So this may take a few days (since I'm out of town). Thanks for pointing the issue out!

Member

rcurtin commented Apr 5, 2016

There's something deeper that's wrong here, but I need to find my copy of the Elliot, Aggoun, and Moore book to figure out exactly what's up. So this may take a few days (since I'm out of town). Thanks for pointing the issue out!

@rcurtin rcurtin added this to the mlpack 2.0.2 milestone Apr 5, 2016

@rcurtin rcurtin self-assigned this Apr 5, 2016

@rcurtin

This comment has been minimized.

Show comment
Hide comment
@rcurtin

rcurtin Apr 12, 2016

Member

d6d38a1 should fix the issue. There was also an issue with how the newInitial matrix was being set. Thanks for reporting the problem!

Member

rcurtin commented Apr 12, 2016

d6d38a1 should fix the issue. There was also an issue with how the newInitial matrix was being set. Thanks for reporting the problem!

@rcurtin rcurtin closed this Apr 12, 2016

@warpuv

This comment has been minimized.

Show comment
Hide comment
@warpuv

warpuv Apr 12, 2016

You forgot to add else condition. initial is not changing if number of sequences equal to one. Thanks for fixing.

warpuv commented Apr 12, 2016

You forgot to add else condition. initial is not changing if number of sequences equal to one. Thanks for fixing.

@rcurtin

This comment has been minimized.

Show comment
Hide comment
@rcurtin

rcurtin Apr 13, 2016

Member

Good catch, thanks for checking my work. e0e7ba5 should fix this.

Member

rcurtin commented Apr 13, 2016

Good catch, thanks for checking my work. e0e7ba5 should fix this.

@rcurtin rcurtin added the R: fixed label Apr 14, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment