Fix batch prefill example script for ragged kv cache#73
Fix batch prefill example script for ragged kv cache#73demandal25 merged 2 commits intoROCm:amd-integrationfrom
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes the batch prefill example script to properly support ragged KV cache functionality. The changes remove naive attention reference comparisons, add support for non-contiguous KV cache testing, improve LSE (log-sum-exp) output verification, and update test configurations.
Key changes:
- Added
contiguous_kvparameter to test non-contiguous memory layouts for KV cache - Enhanced verification to include LSE output comparison when
return_lse=True - Removed naive attention comparisons and unused backend parameter
- Updated test cases with new parameter values
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tests/attention_reference.py | Added clarifying comments about NHD layout for K and V tensor parameters |
| examples/batch_prefill_example.py | Refactored batch prefill functions to support non-contiguous KV cache, added LSE verification, removed naive attention comparisons, and updated test configurations |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Basic ragged KV cache test with causal masking | ||
| batch_prefill_with_ragged_kv_cache_example( | ||
| 12, 54, 37, 8, 8, 128, True, "NONE", 0.0, False | ||
| 12, 54, 37, 8, 8, 64, False, "NONE", 0.0, False |
There was a problem hiding this comment.
The comment on line 427 states 'Basic ragged KV cache test with causal masking' but the function call passes causal=False. Either update the comment to match the actual parameter or change the parameter to True.
| 12, 54, 37, 8, 8, 64, False, "NONE", 0.0, False | |
| 12, 54, 37, 8, 8, 64, True, "NONE", 0.0, False |
Upgrade both the base and CI dockers in `docker/Dockerfile.rocm_ci` to the new version: - rocm6.4 - ubuntu24.04 - py3.12 - torch2.7.1
This PR fixes the batch prefill example script for ragged kv cache. The examples for batch prefill with ragged kv cache in the script are now passing. This reinforces the basic correctness of the PR #50 More exhaustive tests are in the pytest script for batch prefill and should pass to call it a full victory.
Upgrade both the base and CI dockers in `docker/Dockerfile.rocm_ci` to the new version: - rocm6.4 - ubuntu24.04 - py3.12 - torch2.7.1
This PR fixes the batch prefill example script for ragged kv cache. The examples for batch prefill with ragged kv cache in the script are now passing. This reinforces the basic correctness of the PR ROCm#50 More exhaustive tests are in the pytest script for batch prefill and should pass to call it a full victory.
Upgrade both the base and CI dockers in `docker/Dockerfile.rocm_ci` to the new version: - rocm6.4 - ubuntu24.04 - py3.12 - torch2.7.1
This PR fixes the batch prefill example script for ragged kv cache. The examples for batch prefill with ragged kv cache in the script are now passing. This reinforces the basic correctness of the PR ROCm#50 More exhaustive tests are in the pytest script for batch prefill and should pass to call it a full victory.
This PR fixes the batch prefill example script for ragged kv cache. The examples for batch prefill with ragged kv cache in the script are now passing. This reinforces the basic correctness of the PR #50
More exhaustive tests are in the pytest script for batch prefill and should pass to call it a full victory.