[gemm3] Fix EOS token handling for Gemma text-only export#206
Closed
seyeong-han wants to merge 1 commit intohuggingface:mainfrom
Closed
[gemm3] Fix EOS token handling for Gemma text-only export#206seyeong-han wants to merge 1 commit intohuggingface:mainfrom
seyeong-han wants to merge 1 commit intohuggingface:mainfrom
Conversation
…tion Add support for exporting multiple EOS token IDs (`get_eos_ids`) in model metadata to enable proper generation stopping for Gemma models. - Handle cases where config.eos_token_id is already a list vs single int - Detect Gemma models via config.model_type and add <end_of_turn> token (106) - Export get_eos_ids (list) for C++ runner compatibility - Maintain get_eos_id (first ID) for backward compatibility
Author
|
I believe this is not a universal way to adopt the various chat-templates. |
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
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.
Summary
When exporting Gemma models with
--task "text-generation", the C++ text runner wasn't stopping at the<end_of_turn>token (ID 106), only at the<eos>token (ID 1). This caused generation to continue beyond expected stopping points.This diff modifies
save_config_to_constant_methods()to export multiple EOS token IDs viaget_eos_ids, which the C++ runner'skEosIdsmethod already supports.Changes
config.eos_token_idconfig.model_typecontaining "gemma" and automatically add token 106 (<end_of_turn>) to the EOS listget_eos_ids(full list) for C++ runner compatibility andget_eos_id(first element) for backward compatibilityCompatibility
modeling.pyalready checks for bothget_eos_idandget_eos_idsget_eos_ids()function already supports reading a list of EOS tokens via thekEosIdsmethodTest Plan
--task "text-generation"and verify metadata containsget_eos_ids: [1, 106]<end_of_turn>pytest tests/models/test_modeling_gemma3.py -v