-
Notifications
You must be signed in to change notification settings - Fork 104
Add API for setting complex segmentation data #533
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
base: main
Are you sure you want to change the base?
Changes from all commits
1a67974
6435a96
82c4ff9
c363950
d9d3446
e5d462d
e7cee47
23a7ce5
eebb4b4
e6d9a50
616308b
0682914
4e8c40d
ff01a4d
81fd60c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -26,6 +26,73 @@ use icu_segmenter::{ | |||||
| }; | ||||||
| use parley_data::CompositeProps; | ||||||
|
|
||||||
| #[cfg(feature = "runtime-segmenter-data")] | ||||||
| use icu_provider::buf::AsDeserializingBufferProvider; | ||||||
| #[cfg(feature = "runtime-segmenter-data")] | ||||||
| use icu_provider_adapters::fork::{ | ||||||
| ForkByMarkerProvider, MultiForkByErrorProvider, predicates::IdentifierNotFoundPredicate, | ||||||
| }; | ||||||
|
|
||||||
| /// Segmenter model data that can be loaded at runtime. | ||||||
| /// | ||||||
| /// This type wraps binary blob data containing LSTM models or dictionaries for language-specific word/line | ||||||
| /// segmentation. The blobs are exported from `parley_data`, and can be included at compile time or saved as files from | ||||||
| /// `parley_data` at build time and later loaded as files at runtime. | ||||||
| /// | ||||||
| /// # Example | ||||||
| /// | ||||||
| /// ```ignore | ||||||
| /// use parley_data::SegmenterModelData; | ||||||
| /// | ||||||
| /// // Load from a file | ||||||
| /// let blob = std::fs::read("Thai_codepoints_exclusive_model4_heavy.postcard")?; | ||||||
| /// let model = SegmenterModelData::from_blob(blob.into_boxed_slice())?; | ||||||
| /// | ||||||
| /// // Or embed at compile time | ||||||
| /// let model = SegmenterModelData::from_static(parley_data::bundled_models::THAI_LSTM)?; | ||||||
| /// ``` | ||||||
| #[cfg(feature = "runtime-segmenter-data")] | ||||||
| #[derive(Debug)] | ||||||
| pub struct SegmenterModelData { | ||||||
| pub(crate) provider: icu_provider_blob::BlobDataProvider, | ||||||
| } | ||||||
|
|
||||||
| #[cfg(feature = "runtime-segmenter-data")] | ||||||
| impl SegmenterModelData { | ||||||
| /// Creates a new `SegmenterModelData` from an owned blob. | ||||||
| /// | ||||||
| /// The blob should be a postcard-serialized ICU4X data blob from `parley_data`. | ||||||
| pub fn from_blob(blob: alloc::boxed::Box<[u8]>) -> Result<Self, icu_provider::DataError> { | ||||||
| let provider = icu_provider_blob::BlobDataProvider::try_new_from_blob(blob)?; | ||||||
| Ok(Self { provider }) | ||||||
| } | ||||||
|
|
||||||
| /// Creates a new `SegmenterModelData` from a static byte slice. | ||||||
| /// | ||||||
| /// This is useful for embedding model data directly in your binary using `include_bytes!()`. | ||||||
| pub fn from_static(blob: &'static [u8]) -> Result<Self, icu_provider::DataError> { | ||||||
| let provider = icu_provider_blob::BlobDataProvider::try_new_from_static_blob(blob)?; | ||||||
| Ok(Self { provider }) | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| /// The buffer provider for all data loaded at runtime. | ||||||
| #[cfg(feature = "runtime-segmenter-data")] | ||||||
| struct RuntimeBufferProvider { | ||||||
| provider: | ||||||
| MultiForkByErrorProvider<icu_provider_blob::BlobDataProvider, IdentifierNotFoundPredicate>, | ||||||
| segmenter_mode: SegmenterMode, | ||||||
| } | ||||||
|
|
||||||
| #[allow(unused)] | ||||||
| #[derive(Clone, Copy, Debug, PartialEq, Eq)] | ||||||
| pub(crate) enum SegmenterMode { | ||||||
| /// Use LSTM for SE Asian scripts, dictionary for CJK (default). | ||||||
| Auto, | ||||||
| /// Use dictionary for all complex scripts. | ||||||
| Dictionary, | ||||||
| } | ||||||
|
|
||||||
| pub(crate) struct AnalysisDataSources { | ||||||
| grapheme_segmenter: GraphemeClusterSegmenter, | ||||||
| word_segmenter: WordSegmenter, | ||||||
|
|
@@ -36,6 +103,9 @@ pub(crate) struct AnalysisDataSources { | |||||
| brackets: CodePointMapData<BidiMirroringGlyph>, | ||||||
|
|
||||||
| composite: CompositeProps, | ||||||
|
|
||||||
| #[cfg(feature = "runtime-segmenter-data")] | ||||||
| runtime_buffer_provider: Option<RuntimeBufferProvider>, | ||||||
| } | ||||||
|
|
||||||
| #[derive(Default)] | ||||||
|
|
@@ -46,7 +116,13 @@ struct LineSegmenters { | |||||
| } | ||||||
|
|
||||||
| impl LineSegmenters { | ||||||
| fn get(&mut self, word_break_strength: WordBreak) -> LineSegmenterBorrowed<'_> { | ||||||
| fn get( | ||||||
| &mut self, | ||||||
| word_break_strength: WordBreak, | ||||||
| #[cfg(feature = "runtime-segmenter-data")] runtime_buffer_provider: Option< | ||||||
| &RuntimeBufferProvider, | ||||||
| >, | ||||||
| ) -> LineSegmenterBorrowed<'_> { | ||||||
| let segmenter = match word_break_strength { | ||||||
| WordBreak::Normal => &mut self.normal, | ||||||
| WordBreak::KeepAll => &mut self.keep_all, | ||||||
|
|
@@ -62,7 +138,27 @@ impl LineSegmenters { | |||||
| WordBreak::KeepAll => LineBreakWordOption::KeepAll, | ||||||
| }; | ||||||
| line_break_opts.word_option = Some(word_break_strength_icu); | ||||||
| LineSegmenter::try_new_auto_unstable(&PROVIDER, line_break_opts) | ||||||
|
|
||||||
| #[cfg(feature = "runtime-segmenter-data")] | ||||||
| if let Some(&RuntimeBufferProvider { | ||||||
| ref provider, | ||||||
| segmenter_mode, | ||||||
| }) = runtime_buffer_provider | ||||||
| { | ||||||
| let combined = | ||||||
| ForkByMarkerProvider::new(provider.as_deserializing(), &PROVIDER); | ||||||
| return match segmenter_mode { | ||||||
| SegmenterMode::Auto => { | ||||||
| LineSegmenter::try_new_auto_unstable(&combined, line_break_opts) | ||||||
| } | ||||||
| SegmenterMode::Dictionary => { | ||||||
| LineSegmenter::try_new_dictionary_unstable(&combined, line_break_opts) | ||||||
| } | ||||||
| } | ||||||
| .expect("Failed to create LineSegmenter"); | ||||||
| } | ||||||
|
|
||||||
| LineSegmenter::try_new_for_non_complex_scripts_unstable(&PROVIDER, line_break_opts) | ||||||
| .expect("Failed to create LineSegmenter") | ||||||
| }) | ||||||
| .as_borrowed() | ||||||
|
|
@@ -73,7 +169,7 @@ impl AnalysisDataSources { | |||||
| pub(crate) fn new() -> Self { | ||||||
| Self { | ||||||
| grapheme_segmenter: GraphemeClusterSegmenter::try_new_unstable(&PROVIDER).unwrap(), | ||||||
| word_segmenter: WordSegmenter::try_new_lstm_unstable( | ||||||
| word_segmenter: WordSegmenter::try_new_for_non_complex_scripts_unstable( | ||||||
| &PROVIDER, | ||||||
| WordBreakOptions::default(), | ||||||
| ) | ||||||
|
|
@@ -84,6 +180,73 @@ impl AnalysisDataSources { | |||||
| script_short_name: PropertyNamesShort::<Script>::try_new_unstable(&PROVIDER).unwrap(), | ||||||
| brackets: CodePointMapData::<BidiMirroringGlyph>::try_new_unstable(&PROVIDER).unwrap(), | ||||||
| composite: CompositeProps, | ||||||
| #[cfg(feature = "runtime-segmenter-data")] | ||||||
| runtime_buffer_provider: None, | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| #[cfg(feature = "runtime-segmenter-data")] | ||||||
| fn reinitialize_word_segmenter(&mut self) { | ||||||
| let Some(buffer_provider) = self.runtime_buffer_provider.as_ref() else { | ||||||
| return; | ||||||
| }; | ||||||
| // Combine the complex script providers with the baked data for non-complex scripts. | ||||||
| let combined = | ||||||
| ForkByMarkerProvider::new(buffer_provider.provider.as_deserializing(), &PROVIDER); | ||||||
|
|
||||||
| self.word_segmenter = match buffer_provider.segmenter_mode { | ||||||
| SegmenterMode::Auto => { | ||||||
| WordSegmenter::try_new_auto_unstable(&combined, WordBreakOptions::default()) | ||||||
| } | ||||||
| SegmenterMode::Dictionary => { | ||||||
| WordSegmenter::try_new_dictionary_unstable(&combined, WordBreakOptions::default()) | ||||||
| } | ||||||
| } | ||||||
| .expect("Failed to create WordSegmenter with runtime models"); | ||||||
|
|
||||||
| // Clear cached line segmenters; they will be lazily recreated with the new mode. | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why not do the same for all segmenters?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe the line segmenters and word segmenter are the only ones that use complex script data--correct me if I'm wrong. We eagerly reinitialize the word segmenter, and lazily reinitialize the line segmenters.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
yes. I meant why not treat the four segmenters that use complex script data the same
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you should really do it eagerly, because if you have clients passing you arbitrary data, the constructors can fail and you want to return an error instead of panicking. the non-custom constructors ( |
||||||
| self.line_segmenters = LineSegmenters::default(); | ||||||
| } | ||||||
|
|
||||||
| #[cfg(feature = "runtime-segmenter-data")] | ||||||
| pub(crate) fn load_segmenter_models( | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @robertbastian - I was wondering if you would be able to give this code a once over from the perspective of an ICU4X maintainer 🙏
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My understanding is that, since this is a replace operation, consumers cannot easily append models. For example, if a user starts with latin, then adds Thai, then a few minutes later adds Burmese text, when we go to upload the Burmese text, we'll need to re-prepare the Thai model to I'm unsure whether we should support an
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I assumed ICU4X treated things immutably, but it turns out there is a |
||||||
| &mut self, | ||||||
| providers: Vec<icu_provider_blob::BlobDataProvider>, | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd move the
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The reason I collected into a |
||||||
| mode: SegmenterMode, | ||||||
| ) { | ||||||
| // Create a forking buffer provider that combines all blob providers. | ||||||
| let buffer_provider = RuntimeBufferProvider { | ||||||
| provider: MultiForkByErrorProvider::new_with_predicate( | ||||||
| providers, | ||||||
| IdentifierNotFoundPredicate, | ||||||
| ), | ||||||
| segmenter_mode: mode, | ||||||
| }; | ||||||
| self.runtime_buffer_provider = Some(buffer_provider); | ||||||
|
|
||||||
| self.reinitialize_word_segmenter(); | ||||||
| } | ||||||
|
|
||||||
| #[cfg(feature = "runtime-segmenter-data")] | ||||||
| pub(crate) fn append_segmenter_model( | ||||||
| &mut self, | ||||||
| provider: icu_provider_blob::BlobDataProvider, | ||||||
| mode: SegmenterMode, | ||||||
| ) { | ||||||
| match self.runtime_buffer_provider.as_mut() { | ||||||
| None => { | ||||||
| self.load_segmenter_models(alloc::vec![provider], mode); | ||||||
| } | ||||||
| Some(buffer_provider) => { | ||||||
| let cur_mode = buffer_provider.segmenter_mode; | ||||||
| assert_eq!( | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is an undocumented panic. the public append functions only talk about performance issues with duplicated models I'm not sold on the use case of the append methods. when would a client not supply the models all at once?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. cc @taj-p
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
For our own web app, this would give us an opportunity to load in models as the user adds to their content, mostly to minimise memory usage (one of our leading concerns).
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Even then that should be done through the other constructor. If you want to add two new models, calling the append function twice does unnecessary work. |
||||||
| cur_mode, mode, | ||||||
| "Tried to load a {mode:?} segmenter model, but the current segmenters are {cur_mode:?}" | ||||||
| ); | ||||||
|
|
||||||
| buffer_provider.provider.push(provider); | ||||||
| self.reinitialize_word_segmenter(); | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
|
|
@@ -92,6 +255,22 @@ impl AnalysisDataSources { | |||||
| &self.composite | ||||||
| } | ||||||
|
|
||||||
| #[inline(always)] | ||||||
| pub(crate) fn line_segmenter( | ||||||
| &mut self, | ||||||
| word_break_strength: WordBreak, | ||||||
| ) -> LineSegmenterBorrowed<'_> { | ||||||
| #[cfg(feature = "runtime-segmenter-data")] | ||||||
| { | ||||||
| self.line_segmenters | ||||||
| .get(word_break_strength, self.runtime_buffer_provider.as_ref()) | ||||||
| } | ||||||
| #[cfg(not(feature = "runtime-segmenter-data"))] | ||||||
| { | ||||||
| self.line_segmenters.get(word_break_strength) | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| #[inline(always)] | ||||||
| pub(crate) fn grapheme_segmenter(&self) -> GraphemeClusterSegmenterBorrowed<'_> { | ||||||
| self.grapheme_segmenter.as_borrowed() | ||||||
|
|
@@ -354,8 +533,7 @@ pub(crate) fn analyze_text<B: Brush>(lcx: &mut LayoutContext<B>, mut text: &str) | |||||
| if substring_index == 0 && last { | ||||||
| let mut lb_iter = lcx | ||||||
| .analysis_data_sources | ||||||
| .line_segmenters | ||||||
| .get(word_break_strength) | ||||||
| .line_segmenter(word_break_strength) | ||||||
| .segment_str(substring); | ||||||
|
|
||||||
| let _first = lb_iter.next(); | ||||||
|
|
@@ -378,8 +556,7 @@ pub(crate) fn analyze_text<B: Brush>(lcx: &mut LayoutContext<B>, mut text: &str) | |||||
|
|
||||||
| let line_boundaries_iter = lcx | ||||||
| .analysis_data_sources | ||||||
| .line_segmenters | ||||||
| .get(word_break_strength) | ||||||
| .line_segmenter(word_break_strength) | ||||||
| .segment_str(substring); | ||||||
|
|
||||||
| let mut substring_chars = substring.chars(); | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does a consumer know whether they should load in an external segmenter to correctly lay out some input text?
If I understand the API currently, it requires the consumer to decide upfront - either via pre-scanning text prior to Parley or making assumptions based on the type of user and where they may be located.
Since there are only a small size of complex scripts needing this attention, I wonder whether we can use a bitset over a single byte to track this during analysis and provide it back to the caller in
Layout.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might take a bit more work; I'll need to figure out how ICU4X itself determines characters' locales. Not sure if it's a matter of asking each individual model about the character in turn, or if there's an optimization it uses here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ICU4X uses complex data if a code point has the
SAproperty. Inside that code path we use the table from the spec to decide which model to use.I don't think you or your callers should pre-scan the text to determine if complex breaking is required. If you do that, you need to have the data available anyway. Loading the data is very cheap (if you use compiled data, not what this PR is doing), having the data is the problem that some users might want to avoid.