Skip to content
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 collecting counts? #4

Open
dgel opened this issue Sep 23, 2013 · 2 comments
Open

Bug in collecting counts? #4

dgel opened this issue Sep 23, 2013 · 2 comments

Comments

@dgel
Copy link

dgel commented Sep 23, 2013

shouldn't line 225 use j+1 instead of j?

emp_feat += DiagonalAlignment::Feature(j, i, trg.size(), src.size()) * p;

in the surrounding loops, i ranges from 1 to src.size() inclusive and j from 0 to trg.size() exclusive. Further up in the code, j+1 is used as well, and I think the feature calculation assumes the same.

This bug would bias it towards a flatter distribution I think, but I'm not sure.

@redpony
Copy link
Member

redpony commented Sep 23, 2013

You either pay for this with a slightly off-center prior either when j=0 or
j=trg.size() - 1-- I noticed this after I had run all the experiments and
decided not to fix it, but a couple people have pointed it out, so it's
probably worth fixing at some point...

On Mon, Sep 23, 2013 at 12:27 PM, dreugeworst [email protected]:

shouldn't line 225 use j+1 instead of j?

emp_feat += DiagonalAlignment::Feature(j, i, trg.size(), src.size()) * p;

in the surrounding loops, i ranges from 1 to src.size() inclusive and j
from 0 to trg.size() exclusive. Further up in the code, j+1 is used as
well, and I think the feature calculation assumes the same.

This bug would bias it towards a flatter distribution I think, but I'm not
sure.


Reply to this email directly or view it on GitHubhttps://github.com//issues/4
.

@dgel
Copy link
Author

dgel commented Sep 23, 2013

You either pay for this with a slightly off-center prior either when j=0 or j=trg.size() - 1

I'm not sure I quite follow this. Right now when collecting counts the features used are
calculated using 0-based indexing, while the rest of the program uses 1-based indexing.
The diagonal is calculated using the same (1-based). When using j+1 in that bit of code, doesn't it
just fix it to do what the paper says? How does that put the prior off-center?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants