Bug in HMM normalization? #30
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
vene
commented
Sep 22, 2016
| @@ -68,10 +68,10 @@ def fit(self, X, y, lengths): | |||
| final_prob -= logsumexp(final_prob) | |||
There was a problem hiding this comment.
Another observation: final_prob is the same as init_prob here. Should this be Y[end - 1] instead?
|
I suppose so indeed! At first glance the nltk implementation doesn't seem to have an equivalent. Maybe it would be good to write up the specifics of the model implemented here. On September 22, 2016 6:45:46 PM EDT, Mikhail Korobov [email protected] wrote:
Sent from my Android device with K-9 Mail. Please excuse my brevity. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi,
I was trying seqlearn for a realistic problem and the HMM performance was much worse than a structured perceptron (1% vs ~30% F-score). Thinking something was wrong I even tried the implementation in NLTK which performs almost as well as the perceptron.
Upon inspecting the learned parameters I find unexpected results, while NLTK looked sensible. After going through the calculation step by step I think the problem is this; with this patch I get good performance.
I should write a failing test to actually verify that this is correct, and I will try to do so, but it might take a while to find the time...