UTF8 validate and replace invalid in GGUF output decoding #3062
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds UTF-8 validation with replacement mode to the GGUF detokenizer conversion pipeline to handle invalid UTF-8 sequences gracefully during token-to-string decoding.
Key Changes:
- Introduces a UTF8Validate operation between FuzeRagged and StringTensorPack operations in the detokenizer model construction
- Configures the UTF8Validate operation with replace_mode=true to replace invalid UTF-8 sequences rather than failing
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| auto packed_output = create_func("StringTensorPack", outputs_fused_ragged, {}); | ||
| ov::OutputVector inputs_for_utf8_validate(outputs_fused_ragged.begin(), outputs_fused_ragged.end()); | ||
| auto outputs_utf8_validate = create_func("UTF8Validate", inputs_for_utf8_validate, {{"replace_mode", true}}); | ||
| outputs_utf8_validate.insert(outputs_utf8_validate.end(), outputs_fused_ragged.end() - 1, outputs_fused_ragged.end()); |
There was a problem hiding this comment.
This line appears to insert the last element of outputs_fused_ragged into outputs_utf8_validate, but outputs_utf8_validate already contains all elements from outputs_fused_ragged (copied on line 578). This pattern duplicates the last element. If the intent is to append additional outputs (like the original line 577), verify that outputs_fused_ragged contains the correct elements to insert, or if this duplication is intentional for the UTF8Validate operation.
| outputs_utf8_validate.insert(outputs_utf8_validate.end(), outputs_fused_ragged.end() - 1, outputs_fused_ragged.end()); |
There was a problem hiding this comment.
It seems that the insert is not really necessary, indeed.
mzegla
changed the title
github-actions
bot
added
the
category: tokenizers
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| auto packed_output = create_func("StringTensorPack", outputs_fused_ragged, {}); | ||
| ov::OutputVector inputs_for_utf8_validate(outputs_fused_ragged.begin(), outputs_fused_ragged.end()); | ||
| auto outputs_utf8_validate = create_func("UTF8Validate", inputs_for_utf8_validate, {{"replace_mode", true}}); | ||
| outputs_utf8_validate.insert(outputs_utf8_validate.end(), outputs_fused_ragged.end() - 1, outputs_fused_ragged.end()); |
There was a problem hiding this comment.
This line appends the last element of outputs_fused_ragged to outputs_utf8_validate, but outputs_utf8_validate already contains all elements from outputs_fused_ragged (copied in line 578). This results in duplicating the last element. Consider removing this line or clarifying the intended behavior if partial duplication is needed.
| outputs_utf8_validate.insert(outputs_utf8_validate.end(), outputs_fused_ragged.end() - 1, outputs_fused_ragged.end()); |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
5abfc6e
2 participants
Description
Add UTF8 validate with replace mode = true in GGUF detokenizer conversion
CVS-176839
Checklist: