Skip to content
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

add ChunkKV #51

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

add ChunkKV #51

wants to merge 1 commit into from

Conversation

Dominic789654
Copy link

PR Description

Add ChunkKV paper implementation (https://arxiv.org/abs/2502.00299)

Benchmark Results

Results on Ruler (4096) dataset under different compression ratios (0.1, 0.25, 0.5):

Task 0.1 0.25 0.5
cwe 99.66 99.48 97.14
fwe 94.47 94.33 93.53
niah_multikey_1 99.0 95.4 82.4
niah_multikey_2 86.8 65.6 41.8
niah_multikey_3 86.8 60.4 27.4
niah_multiquery 98.85 93.55 76.85
niah_multivalue 98.15 91.05 77.6
niah_single_1 100.0 100.0 100.0
niah_single_2 99.2 97.6 90.0
niah_single_3 89.6 71.8 51.8
qa_1 87.8 86.0 82.6
qa_2 61.0 60.4 76.85
vt 99.88 99.88 77.6
Average 92.40 85.81 75.04

New press checklist

  • I added mypress_press.py in the presses directory
  • I added MyPress in __init__.py
  • I updated the README.md with a 1 liner about my new press in the Available presses section
  • I added my press in the default_presses list in tests/default_presses.py

@maxjeblick maxjeblick self-requested a review February 20, 2025 10:27
Copy link
Collaborator

@maxjeblick maxjeblick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for opening this PR!
The overall code looks good, thanks for adding it. There are several points that I'd ask to address before merging:

  • We prefer to have a new press class (ChunkKVPress) associated with your paper, as your method (selecting whole chunks) differs from ChunkPress's idea (compress each chunk independently). Both methods also have different default parameters (e.g. chunk_length).
  • After introducing a new class, you can add a new test (similar to test_chunk_press). Please do not modify the existing test that uses ExpectedAttentionPress.
  • Please add your class in the corresponding README section (available presses).
  • Most style errors can be fixed my running make format. You can test style locally by make style. Feel free to add # noqa: flags if necessary; e.g. sometimes mypy has issues to resolve tensor attributes correctly.
  • Please don't forget to sign off your commit. See https://github.com/NVIDIA/kvpress/blob/main/CONTRIBUTING.md for additional information.

Feel free to reach out with any questions or comments!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants