-
Notifications
You must be signed in to change notification settings - Fork 603
tweak OpenAI Harmony autoguess developer prefix and assistant end token #1673
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
tweak OpenAI Harmony autoguess developer prefix and assistant end token #1673
Conversation
|
OpenAI's docs are inconsistent. Some examples they use
|
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.
do let me know the decision.
Edit: maybe <|return|> is more like an EOS?
Good catch. Damn. Will have to dig a bit deeper. So far, though, the .jinja template as well as the Ollama template at https://ollama.com/library/gpt-oss:20b/blobs/51468a0fd901 both seem to suggest Another hint as to why this
from https://huggingface.co/blog/welcome-openai-gpt-oss where they talk about the fact you should only train on/unmask the very last message due to how the thinking stuff works. Maybe Either way, I think using |
|
Yeah, that example looks broken to me.
It has no place in a chat log other than to tell the inference engine that the model is done.
There may be some subtle distinction in their minds between "end of the turn" and "finished completion" which has to do with tool calling or things, but they're not being very obvious about it:
The conclusion I think is: if the front end does not add {#- <|return|> indicates the end of generation, but <|end|> does not #}
{#- <|return|> should never be an input to the model, but we include it as the final token #}
{#- when training, so the model learns to emit it. #}
{{- "<|start|>assistant<|channel|>final<|message|>" + message.content + "<|return|>" }} |
|
alright lets merge this |



# Instructions\n\nprefix.<|return|>token is used for training, and we want to use<|end|>for the generic case. See details below.The .jinja template has this:
i.e. it prefixes the developer (system) message with
# Instructions\n\n.(Caught in #1659, but pushing as a separate PR as I don't know if that PR will survive.)
Edit: after re-running the test, I'm also seeing this
which indicates the assistant end prefix should be
<|end|>and not<|return|>. I'm not clear on the difference atm.For
[{"role":"developer","content":"hi"},{"role":"user","content":"hello"},{"role":"assistant","content":"e"},{"role":"user","content":"o"}], the chat template outputs (withadd_generation_prompt=True):(
add_generation_prompt=False):A-HA! This thing is specifically for training: when the assistant is the last role and there is no add_generation_prompt, it assumes you are generating training examples, and it uses
<|return|>instead of<|end|>, as described in-- the above with the last user turn removed and
add_generation_prompt=False:Nifty. But it does mean the adapter should use
<|end|>, I'd say.