krickert opened a new pull request, #1206: URL: https://github.com/apache/opennlp/pull/1206
Fixes the sites listed in OPENNLP-1904. Several code paths fold case with the JVM default locale. In the lemmatizer this is not cosmetic: the folded strings become shortest-edit-script labels that `encodeLemmas` writes **into the trained model**, and `decodeLemmas` folds again with whatever locale the consumer runs under. A model trained on a Turkish JVM carries labels no other machine reproduces, and an English-trained model emits corrupted lemmas when served on one. Nothing logs and nothing throws. ### Reproduction Assertion failures observed against unfixed code, under `Locale.setDefault` of `tr` (or `de_DE` for the report cases): ``` LemmatizerME.decodeLemmas expected <mouse> but was <mıuse> StringUtil.getShortestEditScript expected <D0s> but was <R6ıiD0s> LemmatizerME.encodeLemmas expected <R2ioR1cuI1s> but was <R2ıoR1cuI1s> DictionaryLemmatizer expected <Illinois> but was <O> TrainingParameters.setParams expected <50> but was <100> ``` The last one is a separate user-visible bug in the same family: `setParams` computes the flag name as `"Iterations".toLowerCase()`, which under `tr` yields `-ıterations`, so `TokenizerTrainerTool` silently ignores `-iterations` and trains with the default. ### Why `Locale.ROOT` and not `StringUtil.toLowerCase` This is the load-bearing decision and the likely review question. `StringUtil.toLowerCase` maps code points through `Character.toLowerCase`. That is context free and disagrees with `String.toLowerCase(Locale.ROOT)` on context-sensitive mappings (Greek final sigma: `ΟΔΟΣ` folds to `οδοσ` rather than `οδος`) and on one-to-many mappings such as `ß`. Every shipped model was produced on an English-locale JVM, whose fold is byte-identical to `Locale.ROOT`. So `Locale.ROOT` keeps existing artifacts working unchanged, whereas the code-point fold would silently alter them. **No model regeneration is required.** `StringUtilLocaleTest` carries a guard that fails if this is later switched to the code-point fold for consistency. ### Behaviour change for release notes `DetailedFMeasureListener.createReport()` and `FineGrainedReportListener` now always render with `Locale.ROOT`, so CLI eval report output is no longer localized. This was chosen so reports are reproducible, diffable and machine-parseable across machines. The explicit `createReport(Locale)` overload is untouched for callers who want localization. ### Tests There was no locale test coverage anywhere in the tree before this: no `Locale.setDefault` in main or test sources. Each new test was verified to fail before the fix and pass after. Two gaps, both deliberate: - `AbstractModel.DECIMAL_FORMAT` is fixed without a red/green test. The formatter is a `static final` resolved at class initialization, so a `setDefault` test passes both before and after and proves nothing. - `StringUtilLocaleTest.testGetShortestEditScriptUsesRootFoldingNotCodePointFolding` is a guard, not a regression test. It passes before and after; its purpose is to break on a future change. Note for anyone adding more: the root `pom.xml` surefire `argLine`s do not forward `-Duser.language`, so `mvn -Duser.language=tr test` does not reach the forked test JVM. Full 23-module reactor: BUILD SUCCESS, zero failures, with checkstyle, rat and forbiddenapis active. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
