🏷️ Add chunker types #762
Merged
🏷️ Add chunker types #762
Conversation
This file contains hidden or 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
Signed-off-by: Evaline Ju <69598118+evaline-ju@users.noreply.github.com> Co-authored-by: gkumbhat <kumbhat.gaurav@gmail.com>
Signed-off-by: Evaline Ju <69598118+evaline-ju@users.noreply.github.com>
gabe-l-hart
reviewed
Sep 4, 2024
| # Below 2 represent pointer from the input stream. | ||
| # These are different from start and processed_index index returned from | ||
| # TokenizationStreamResult, which refers to the char span | ||
| input_start_index: Annotated[int, FieldNumber(5)] |
There was a problem hiding this comment.
Small thought: It might be good to use a fully different set of field numbers for derived messages like this so that if additional fields are added to the parent, they don't need to be aware of all children. I'd suggest going with something like 20 and 21 or something like that.
There was a problem hiding this comment.
good point ^^.. otherwise, there may be cases where parent adds new field and now things are overlapping
There was a problem hiding this comment.
sure, technically this will be a breaking change for current usage (from where this was ported from) but this can be accounted for
Signed-off-by: Evaline Ju <69598118+evaline-ju@users.noreply.github.com>
This was referenced Sep 5, 2024
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.
What this PR does / why we need it:
"Chunkers" can be thought of as servers that “chunk” the target modality. For text, this can look like tokenizers or sentence splitters. ref. https://github.com/foundation-model-stack/fms-guardrails-orchestrator/blob/main/docs/architecture/adrs/001-orchestrator.md
This PR essentially "open-sources" the chunker stream result and chunker task, to enable chunker implementations at least with the chunker task.
The chunker task and stream result are very similar to the existing "tokenization" task and stream result, with the addition of tracking indices in the stream case. This helps track information in the case of what tokens/text (say from a text generation server) map to which sentences in a chunker result, if the chunker is a sentence splitter.
Special notes for your reviewer:
If applicable: