-
Notifications
You must be signed in to change notification settings - Fork 13.4k
server : disable context shift by default #15416
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
Conversation
@ggerganov I'm looking for ressources about the behaviour when context overflows. I was planning to conduct experiments using this --context-shift along with --keep N option (still not sure if this one is relevant) and --ctx-size smaller than training context. What should I get from this change ? Is there a link with attention sink recently supported in llama.cpp ? Is this --context-shift option unrelevant for instruct fine-tuned model ? |
This only changes the default behavior, instead of having context shift on by default, it's now off by default. You can manually enable it. |
server.enable_ctx_shift = True | ||
server.start() | ||
server.enable_ctx_shift = False |
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.
@ngxson I noticed that the server parameters are stateful - i.e. if we change a parameter in one test, it will remain changed for the rest of the tests. This is the reason I do it like this here.
Is there a better way to set the parameter just for the scope of the current test?
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.
It could be possible that the scope=module is the problem. Could you try removing it? (While keeping auto_use)
I was a bit confused about the notion of scope in pytest
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.
Thanks - this seems to work.
@GuillaumeBruand The context shift is difficult to handle with formatted endpoints such as |
Thanks for the insight, I'll go on with this PR and let it disabled for my experiments. |
Hi @ggerganov , the help msg about |
Context shift was a useful feature in the past with pre-trained models and the raw
/completions
API. But today, it is causing a lot of confusion, so it is better to disable it by default. Can be re-enabled with--context-shift
CLI arg.