Use Intl.Segmenter instead of ssplit for segmentation in WASM builds
#945
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
nordzilla
force-pushed
the
wasm-intl-segmenter
branch
from
196c76b to
eef64a9
Compare
nordzilla
force-pushed
the
wasm-intl-segmenter
branch
15 times, most recently
from
1caa82e to
bbf2551
Compare
gregtatum
reviewed
Dec 2, 2024
inference/wasm/tests/test-cases/translate-html-multi-paragraph-no-pivot.test.mjs
Show resolved
Hide resolved
inference/wasm/tests/test-cases/translate-html-multi-paragraph-no-pivot.test.mjs
Show resolved
Hide resolved
nordzilla
force-pushed
the
wasm-intl-segmenter
branch
from
bbf2551 to
86f4bab
Compare
|
Thanks for the review! I've made changes according to most of your feedback so far (changeset). Any comments that are not yet resolved have replies. Happy to keep iterating on this as needed! |
nordzilla
force-pushed
the
wasm-intl-segmenter
branch
from
86f4bab to
428ab1a
Compare
gregtatum
approved these changes
Dec 3, 2024
|
Ah I missed the perf numbers at the end. The only potential follow-up would be to run it on a text heavy wikipedia page, but I'm happy with a profile on the benchmark. |
This fixes an oversight from a previous PR when `inference-engine` was renamed to `inference`, however the path was not updated in `CODEOWNERS`.
This is a miscellaneous change to the eslint config that now allows different string types based on whether certain types of quotes need to be escaped within the string.
This commit adds a --force-rebuild flag to the WASM build commands that will trigger a rebuild without having to fully clobber and start over.
This commit fixes miscellaneous formatting that I noticed looked misaligned in the terminal. For some reason, some emojis need two spaces after them, when other emojis only need one space to achieve the same alignment.
This commit renames `appendEndingWhitespace` to `handleEndingWhitespace`, because the whitespace logic will be made more complex by this PR, and whitspace is no longer guaranteed to be appended.
This commit adds the capability for several of the C++ classes to register either a source language tag or a target language tag (depending on their needs). I had experimented with changing the constructors themselves, but mtaintaining backward compatibility got messy very quickly with native builds continuing to use `ssplit` and WASM builds now using `Intl.Segmenter`. The least-invasive and cleanest-to-implement compromise that I came up with was to add WASM-specific functionality to register the language tags for classes after construction.
This is the largest commit of the stack, and likely the one to pay the most attention to. In addition to utilizing `Intl.Segmenter` instead of `ssplit` when segmenting text in WASM builds, this patch also necessarily modifies the logic of how whitespce is handled during translations. We now have to concern ourselves with whether the source language and/or target language utilize whitespace between sentences or omit whitespace between sentences. For example: * When translating from Chinese to English, then whitespace must be added between sentences. * When translating from English to Chinese, then whitespace must be removed between sentences. * When translating form Chinese to Japanese, then whitesapce must be inserted between sentences for the English pivot, and then removed for the final output.
This commit entirely removes the build dependency on `ssplit` when building the WASM target. This actually ultimately reduces the size of the compiled WASM binary from 5.01 MB to 4.73 MB.
Part 1 of 2 This commit updates the WASM bindings to take the source language and target language tags in order to construct the TranslationModels that now utilize the locale-specific `Intl.Segmenter`. This effectively takes the `LanguageTranslationModelFiles` object and makes that a sub-object of `TranslationModelPayload`, which includes the language tags as well as the files. This hierarchical separation is ideal, because the `LanguageTranslationModelFiles` object is designed to be iterated over and chunked into aligned memory, where as the language tags are plain strings that are distinctly separate in the way that they are handled.
Part 2 of 2 This commit reworks the TranslationsEngine worker code to utilize the new bindings implemented in the previous commit.
This commit introduces extra logic to the text cleaning that purposely inserts whitespace into CJK text to trick the segmenter into doing the right thing. See the in-code comment for more context.
This commit adds our work-in-progress `zhen` model to the repository for use in testing.
This commit adds several test cases for translating from Chinese into other languages, which will both guard against regressions and demonstrate correct segmentation behavior.
Part 1 of 2 The final two commits of this stack may be slightly controversial. We do not currently have a viable `enzh` model, even for testing purposes, however, I need to test the functionality of removing whitespace between sentences for target languages that require it. This patch adds our `enes` models under the `enzh` directory, which will trick the implementation into translating into "Chinese" with a Spanish output. The key difference is that the Spanish output should not include spaces between sentences, which is, in my opinion, good enough for testing in the interim.
Part 2 of 2 This patch adds test cases for translating into "Chinese", which at present, is actually a Spanish translation that omits spaces between sentences.
nordzilla
force-pushed
the
wasm-intl-segmenter
branch
from
428ab1a to
63c4022
Compare
|
I was able to run our sentence splitting benchmark for ICU4X, see "2022 (mixed) avg" sheet for a summary and "2022 (mixed)" sheet for details on each tested language: https://docs.google.com/spreadsheets/d/1mGJ9MSyMlsK0EUDRC2J50uxApiti3ggnlrzAWn8rkMg/edit?usp=sharing Not the best, but also no significant flaws. |
This is really great @ZJaume, thank you for putting this together. |
ZJaume
pushed a commit
that referenced
this pull request
Jan 16, 2025
…lds (#945) * Fix inference in CODEOWNERS file This fixes an oversight from a previous PR when `inference-engine` was renamed to `inference`, however the path was not updated in `CODEOWNERS`. * Improve eslint string-formatting configuration This is a miscellaneous change to the eslint config that now allows different string types based on whether certain types of quotes need to be escaped within the string. * Add a --force-rebuild flag to WASM build commands This commit adds a --force-rebuild flag to the WASM build commands that will trigger a rebuild without having to fully clobber and start over. * Fix misc. formatting in build-bergamot.py This commit fixes miscellaneous formatting that I noticed looked misaligned in the terminal. For some reason, some emojis need two spaces after them, when other emojis only need one space to achieve the same alignment. * Rename `appendEndingWhitespace` This commit renames `appendEndingWhitespace` to `handleEndingWhitespace`, because the whitespace logic will be made more complex by this PR, and whitspace is no longer guaranteed to be appended. * Add capability to register languages This commit adds the capability for several of the C++ classes to register either a source language tag or a target language tag (depending on their needs). I had experimented with changing the constructors themselves, but mtaintaining backward compatibility got messy very quickly with native builds continuing to use `ssplit` and WASM builds now using `Intl.Segmenter`. The least-invasive and cleanest-to-implement compromise that I came up with was to add WASM-specific functionality to register the language tags for classes after construction. * Implement WASM segmentation with `Intl.Segmenter` This is the largest commit of the stack, and likely the one to pay the most attention to. In addition to utilizing `Intl.Segmenter` instead of `ssplit` when segmenting text in WASM builds, this patch also necessarily modifies the logic of how whitespce is handled during translations. We now have to concern ourselves with whether the source language and/or target language utilize whitespace between sentences or omit whitespace between sentences. For example: * When translating from Chinese to English, then whitespace must be added between sentences. * When translating from English to Chinese, then whitespace must be removed between sentences. * When translating form Chinese to Japanese, then whitesapce must be inserted between sentences for the English pivot, and then removed for the final output. * Remove WASM dependency on ssplit This commit entirely removes the build dependency on `ssplit` when building the WASM target. This actually ultimately reduces the size of the compiled WASM binary from 5.01 MB to 4.73 MB. * Bump Bergamot Version 0.4.5 => 0.5.0 * Update WASM Bindings Part 1 of 2 This commit updates the WASM bindings to take the source language and target language tags in order to construct the TranslationModels that now utilize the locale-specific `Intl.Segmenter`. This effectively takes the `LanguageTranslationModelFiles` object and makes that a sub-object of `TranslationModelPayload`, which includes the language tags as well as the files. This hierarchical separation is ideal, because the `LanguageTranslationModelFiles` object is designed to be iterated over and chunked into aligned memory, where as the language tags are plain strings that are distinctly separate in the way that they are handled. * Rework TranslationsEngine to utilize new bindings Part 2 of 2 This commit reworks the TranslationsEngine worker code to utilize the new bindings implemented in the previous commit. * Insert whitespace between full-width punctuation and opening quotes This commit introduces extra logic to the text cleaning that purposely inserts whitespace into CJK text to trick the segmenter into doing the right thing. See the in-code comment for more context. * Add `zhen` test model files This commit adds our work-in-progress `zhen` model to the repository for use in testing. * Add test cases for testing `zhen` models. This commit adds several test cases for translating from Chinese into other languages, which will both guard against regressions and demonstrate correct segmentation behavior. * Add temporary `enzh` models for testing Part 1 of 2 The final two commits of this stack may be slightly controversial. We do not currently have a viable `enzh` model, even for testing purposes, however, I need to test the functionality of removing whitespace between sentences for target languages that require it. This patch adds our `enes` models under the `enzh` directory, which will trick the implementation into translating into "Chinese" with a Spanish output. The key difference is that the Spanish output should not include spaces between sentences, which is, in my opinion, good enough for testing in the interim. * Add makeshift `enzh` tests Part 2 of 2 This patch adds test cases for translating into "Chinese", which at present, is actually a Spanish translation that omits spaces between sentences.
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.
Description
Note
I have two follow-up issues filed for after this patch lands. Please take them into consideration when reviewing.
This pull request fully removes the dependency on ssplit from WASM builds of the inference engine, instead utilizing the Intl.Segmenter implementation within the JavaScript context in which the WASM is being run. This offers a number of benefits.
Correctness Improvements
Intl.Segmenteris created from a specific locale and knows how to segment text according to that language.ssplithas limited support for CJK segmentation and is a one-size-fits-all segmentation solution that overall works well, but is not robust enough in its current state for all of the languages we want to support.As an experiment, I cherry-picked the new test cases from this pull request onto the main branch and ran them.
Note
While the
Intl.Segmenterimproves correctness for languages that do not utilize whitespace between sentences, I have no reason to believe that it has regressed the preexisting behavior of translating between two languages that utilize whitespace between sentences. As a check, I did a diff of the HTML output of the benchmark page after doing a pivot translation from Spanish to French, and the output was identical both before and after the changes in this PR.Upstream Updates
By moving to
Intl.Segmenterwe also get free, standardized updates whenever its algorithms are improved as part of the JavaScript API.Compiled WASM Binary Size Reduction
By fully removing
ssplitas a dependency from the WASM build, we reduce the size of our compiled WASM binary:Similar Performance
The new implementation actually showed improved performance on the benchmark page. However, this is negligible with a sample size of 1, and is almost certainly within the range of statistical noise.
Nonetheless, I feel encouraged that there is no major or obvious regression coupled with these changes.
Note
esfrtranslation benchmarks were profiled (links in table).esenssplitesenIntl.SegmenteresfrssplitesfrIntl.Segmenter