Add secondary longest-suffix exemplar guessing for Serbian#197
Conversation
4fba7f7 to
cc389e5
Compare
grhoten
left a comment
There was a problem hiding this comment.
If you're going this route, you might want to enable ExemplarTest#testExemplarsInDictionary to ensure that your new table is valid. The TODO can be to enable it for the other languages.
The other suffix tables are a work in progress. Either the missing words need to be added, or the table needs to be regenerated.
| @@ -0,0 +1,19 @@ | |||
| ак,=,изузетак | |||
There was a problem hiding this comment.
Was this file manually generated or generated from genExemplars? It seems a little short.
There was a problem hiding this comment.
FYI the other tables are mostly generated with some small manual tweaking.
There was a problem hiding this comment.
This was manually generated - I'll look into genExemplars, thanks for the pointer.
| // Primary guess: Rule based inflection for nouns. | ||
| ::std::u16string ruleResult = inflectWithRule(constraints, displayString); | ||
| if (ruleResult != displayString) { | ||
| displayString = ruleResult; | ||
| } else { | ||
| // Secondary guess: Longest suffix exemplar guessing. |
There was a problem hiding this comment.
This is an interesting combination. Why not go all in for one or the other? What is the advantage of one over the other?
There was a problem hiding this comment.
Rule based inflection is going to give the correct answer for words not in the dictionary (with minor exceptions).
Some of the groups are hard to implement, or require more grammatical information to do properly - the longest suffix match is replacement for those for now. I expect the results to be occasionally incorrect, so it's the least preferred approach of the 3 (dictionary, rules, suffix guess) wrt correctness, but easy to implement :).
There was a problem hiding this comment.
So I actually went and implemented the other 3 cases and it wasn't that bad.
I hit a snag on this one as the suffix matching would need to be hand curated to avoid some of the special cases and with the PR #198 this PR is not needed.
Let's discuss it tomorrow.
| auto countString = GrammarSynthesizerUtil::getFeatureValue(constraints, numberFeature); | ||
| auto caseString = GrammarSynthesizerUtil::getFeatureValue(constraints, caseFeature); | ||
| auto genderString = GrammarSynthesizerUtil::getFeatureValue(constraints, genderFeature); | ||
| std::vector<::std::u16string> constraintsVec; | ||
| if (!countString.empty()) constraintsVec.push_back(countString); | ||
| if (!caseString.empty() && caseString != GrammemeConstants::CASE_NOMINATIVE) constraintsVec.push_back(caseString); | ||
| if (!genderString.empty()) constraintsVec.push_back(genderString); |
There was a problem hiding this comment.
I would have expected these values to already be derived. Why are they being queried here?
Also please don't inline if statements. It makes it hard to debug, and get decent code coverage metrics.
I added secondary longest-suffix exemplar guessing for Serbian nouns when rule-based guessing doesn't inflect a word missing from the dictionary.
Key changes:
suffix_sr.csvmapping common Serbian Cyrillic noun endings (-ак,-њак,-ац,-ник,-ач,-ар,-бус,-ан,-ош,-иша,-ица,-ија,-ка,-о,-бло,-е,-ње,-ме) to Cyrillic dictionary exemplars.SrGrammarSynthesizer_SrDisplayFunctionto fall back tosuffixToExemplar+inflectExemplarifinflectWithRulereturns the uninflected word.sr.xmlcovering unknown words with diverse endings.