Skip to content
This repository has been archived by the owner on Dec 13, 2021. It is now read-only.

Commit

Permalink
bugfix: unknown owner (a string) was being passed in as an alignment …
Browse files Browse the repository at this point in the history 
…string
  • Loading branch information
@mjpost
mjpost committed Jun 23, 2016
1 parent 6aef63d commit 748cf32
Showing 1 changed file with 1 addition and 2 deletions.
3 changes: 1 addition & 2 deletions src/main/java/org/apache/joshua/decoder/phrase/PhraseTable.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,7 @@ public void addOOVRules(int sourceWord, List<FeatureFunction> featureFunctions)
: sourceWord;

int nt_i = Vocabulary.id("[X]");
Rule oovRule = new Rule(nt_i, new int[] { nt_i, sourceWord },
new int[] { -1, targetWord }, "", 1, UNKNOWN_OWNER);
Rule oovRule = new Rule(nt_i, new int[] { nt_i, sourceWord }, new int[] { -1, targetWord }, "", 1);
addRule(oovRule);
oovRule.estimateRuleCost(featureFunctions);

Expand Down

5 comments on commit 748cf32

@fhieber
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Matt,
Can you comment how this was a bug and what this change changes? Using the Rule constructor without an owner will just assign the Unknown owner id to it in the constructor itself afaik. So what should the owner be here?

@mjpost
Copy link
Contributor Author

@mjpost mjpost commented on 748cf32 Jun 23, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure — UNKNOWN_OWNER is a String, so the constructor that gets called is this one, which interprets the last argument as an alignment, not an owner.

@fhieber
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, right! Thanks! Should have looked more closely.

@mjpost
Copy link
Contributor Author

@mjpost mjpost commented on 748cf32 Jun 23, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No problem, it didn't get triggered until I had a small case where I actually asked for the alignments on output.

I suppose we could also have fixed this by passing UKNOWN_OWNER_ID, we can switch it if you prefer.

@fhieber
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No its totally fine. Thanks for the fix!

Please sign in to comment.