-
Notifications
You must be signed in to change notification settings - Fork 26
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
Integrating CriticalKV with Existing Methods #46
Conversation
Signed-off-by: yuanfeng <[email protected]>
Signed-off-by: yuanfeng <[email protected]>
Signed-off-by: yuanfeng <[email protected]>
@FFY0 thanks for opening the PR, as you can see you have style errors. Please fix them (this can be done automatically if you have black or something like this). You can run flake8 locally to check you fixed the errors |
also, please open the PR for main, not to on my branch ^^ |
Signed-off-by: yuanfeng <[email protected]>
LOL, OK!😂 |
two remaining style errors:
|
This error wasn’t detected by flake8 when I ran it locally. I checked the corresponding file, and line 42 is just a blank line, so a "missing whitespace" issue shouldn’t appear there. It seems a bit strange. |
@maxjeblick I don't understand why there are style errors in files @FFY0 did not change... |
Style checks are currently failing, see #48 |
@FFY0 Style errors have been fixed, you can merge main into your branch. |
@FFY0, @maxjeblick fixed the style issue, please merge with main and commit the changes. Don't forget to sign-off your commits. |
PR description
Support for a new work: CriticalKV Openreview ArXiv.
Fixes #45
This method introduces a novel metric for KV cache entry selection from an output perturbation perspective, which can be seamlessly integrated to enhance existing techniques. When integrating SnapKV, Expected Attention, and Ada-KV, the preliminary results on the 4K ruler with Llama-3.1-8B are shown below.