-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Regarding setup_chat_format
overwriting existing special tokens
#1819
Comments
If you fine-tuning model with full parameters it usually does not cause problems. But if you fine-tuning model with peft method such as Lora, it may cause problems |
@AIR-hl I see. I can see why that is the case, as full-parameter tuning updates the embeddings of the new token in the input embedding layer. I have one closely related question: how is training model with chat template enabled different from using Update: actually there might still be a problem. If |
any idea? |
were you using lora to fine-tune your model? |
@zyzhang1130 In fact, the |
@zyzhang1130 yes
|
Lora doesn't play well with |
yes, actually I had to implement a version of it myself to handle those issues. Other than formatting, |
This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread. |
because of https://github.com/huggingface/trl/blob/2860ce5091e689bab167454453e9ddbe2337de3d/trl/models/utils.py#L90C2-L90C48,
setup_chat_format
will overwrite the existingeos_token
,pad_token
, andbos_token
. Why need to do such thing? My understanding is thatsetup_chat_format
is supposed to facilitate instruction fine-tuning by adding special tokens to indicate the beginning and end of the turn of the dialogues of the user and model (i.e., assistant). If this is the case, then simply addingchat_format.bos_token
,chat_format.eos_token
should suffice right? The issue with overwriting existingeos_token
,pad_token
, andbos_token
is that the pre-trained models we are supposed to finetune were trained on these tokens, and hence overwriting them would certainly cause problems right?The text was updated successfully, but these errors were encountered: