Repository navigation
Conversation
Signed-off-by: JartX <sagformas@epdcenter.es>
Signed-off-by: JartX <sagformas@epdcenter.es>
Signed-off-by: JartX <sagformas@epdcenter.es>
Signed-off-by: JartX <sagformas@epdcenter.es>
Signed-off-by: JartX <sagformas@epdcenter.es>
Signed-off-by: JartX <sagformas@epdcenter.es>
Signed-off-by: JartX <sagformas@epdcenter.es>
Signed-off-by: JartX <sagformas@epdcenter.es>
Signed-off-by: JartX <sagformas@epdcenter.es>
Signed-off-by: JartX <sagformas@epdcenter.es>
Signed-off-by: JartX <sagformas@epdcenter.es>
Signed-off-by: JartX <sagformas@epdcenter.es>
Signed-off-by: JartX <sagformas@epdcenter.es>
Signed-off-by: JartX <sagformas@epdcenter.es>
Signed-off-by: JartX <sagformas@epdcenter.es>
Signed-off-by: JartX <sagformas@epdcenter.es>
Signed-off-by: JartX <sagformas@epdcenter.es>
|
Hi @tdoublep Could you please check it again? Only the int4 kernel remains, and the rest has been unified. |
tdoublep
left a comment
There was a problem hiding this comment.
Thanks! I have some minor comments to try and minimize diff, but I think we are nearly there.
| self._is_per_token_head = kv_cache_spec.kv_quant_mode.is_per_token_head | ||
| if self._is_per_token_head: | ||
| self._init_reorder_batch_threshold(1, supports_spec_as_decode=False) | ||
|
|
There was a problem hiding this comment.
Yeah, this one's just leftover. It used to back a flash-attn varlen fastpath that needed decodes up front, but that path is gone now that everything goes through unified_attention. Removed it.
| seq_lens_cpu = common_attn_metadata._seq_lens_cpu | ||
| qsl_cpu = ( | ||
| common_attn_metadata.query_start_loc_cpu | ||
| if seq_lens_cpu is not None | ||
| else None | ||
| ) | ||
|
|
There was a problem hiding this comment.
Do we really need this subdir - can't we just have the kernel under ops?
There was a problem hiding this comment.
Yeah, fair — it didn't really earn its own package. Moved it up to ops/int4_per_token_head.py and dropped the subdir. Done in the latest push.
| K = _cast_kv_tile(K_load, Q, k_scale, KV_QUANT_MODE) | ||
| V = _cast_kv_tile(V_load, Q, v_scale, KV_QUANT_MODE) |
There was a problem hiding this comment.
What is reason for moving this?
There was a problem hiding this comment.
Good eye, nothing actually calls it, it's a leftover from an earlier pass. Removed, helpers file is back to matching main. I left it orphaned during the refactor
|
|
||
|
|
||
| @triton.jit | ||
| def cast_kv_tile(data, Q, tensor_scale, KV_QUANT_MODE: tl.constexpr): |
There was a problem hiding this comment.
Maybe I'm missing something but where is this used?
| k_scale_cache: torch.Tensor, # [num_blocks, block_size, num_kv_heads] float32 | ||
| v_scale_cache: torch.Tensor, # [num_blocks, block_size, num_kv_heads] float32 | ||
| slot_mapping: torch.Tensor, # [num_tokens] | ||
| *, |
| full_block_f32 = 2 * kv_half_bytes // 4 | ||
| slot_f32 = nkv * padded_hs * dtype_sz // 4 | ||
| head_f32 = padded_hs * dtype_sz // 4 | ||
| scale_off_f32 = hs * dtype_sz // 4 |
| head_f32 = padded_hs * dtype_sz // 4 | ||
| scale_off_f32 = hs * dtype_sz // 4 | ||
|
|
||
| # K scales: kv_half=0 |
| ) | ||
| self._k_scale_cache.fill_(1.0) | ||
|
|
||
| # V scales: kv_half=1, offset by kv_half_bytes |
| ) | ||
| @pytest.mark.parametrize( | ||
| "base_model,test_model", | ||
| "base_model,test_model,kv_cache_dtype", |
There was a problem hiding this comment.
How much does this increase the test time?
There was a problem hiding this comment.
Switched int4 to Llama-3.2-1B-Instruct so it doesn't drag in the 8B anymore, now it's just one extra dtype on the existing param.I originally moved it up to keep it in line with int2 (which genuinely fell apart at 1B), but int4 should be fine on the small model. If logprobs end up too tight we'll bump just that case back to 8B
- Remove leftover reorder_batch_threshold init (dead since per-token-head moved to unified_attention) - Remove unused seq_lens_cpu / query_start_loc_cpu metadata fields - Flatten triton_quant_kv/ subdir into ops/int4_per_token_head.py - Revert _cast_kv_tile move in triton_unified_attention - Drop unused cast_kv_tile helper (triton_attention_helpers back to main) - Drop keyword-only * from reshape_and_cache_per_token_head_quant - Restore original _ensure_scale_caches comments - Use Llama-3.2-1B for the int4 e2e case (was Meta-Llama-3-8B) Signed-off-by: JartX <sagformas@epdcenter.es>
|
Thanks @tdoublep, really good catches . Changes applied
|
tdoublep
left a comment
There was a problem hiding this comment.
Just did a last pass through the tests. Please revert the change of introducing "device" everywhere, it creates huge churn for no reason that I can see.
| ) | ||
|
|
||
| torch.set_default_device(DEVICE_TYPE) | ||
| device = "cuda" |
There was a problem hiding this comment.
why do we need to pass the device around in addition to setting the default device?
|
|
||
| cache_dtype: torch.dtype # torch.int8 or FP8_DTYPE | ||
| kv_cache_dtype_str: str # "int8_per_token_head" or "fp8_per_token_head" | ||
| cache_dtype: torch.dtype # torch.uint8, torch.int8, or FP8_DTYPE |
There was a problem hiding this comment.
Why does this specific comment change?
|
|
||
| set_random_seed(seed) | ||
| torch.set_default_device(DEVICE_TYPE) | ||
| device = "cuda" |
| if is_int4: | ||
| deq_atol, deq_rtol = 0.5, 0.5 | ||
| else: | ||
| deq_atol, deq_rtol = 0.1, 0.1 |
There was a problem hiding this comment.
where is deq_atol and deq_rtol used in non-int4 path?
|
|
||
| torch.set_default_device(DEVICE_TYPE) | ||
| set_random_seed(42) | ||
| device = "cuda" |
| for i in range(num_tokens): | ||
| blk = i // block_size | ||
| off = i % block_size | ||
|
|
|
|
||
| torch.set_default_device(DEVICE_TYPE) | ||
| set_random_seed(0) | ||
| device = "cuda" |
- Drop the device="cuda" / device=device churn (set_default_device already handles placement; was also hardcoding cuda) - Restore original QuantConfig dtype comments - Move int4 tolerance into the int4 branch (deq_atol/deq_rtol were unused in the int8/fp8 path) - Restore blank line in round-trip loop; recollapse calls to match main Signed-off-by: JartX <sagformas@epdcenter.es>
|
Thanks again @tdoublep, all good catches Thanks so much for the thorough reviews. Learned quite a bit going through them — good catches all around. I have pushed the fixes:
|
Two CI failures from stale call sites after merging main: - int4_per_token_head kernel called compute_kv_seq_mask without the seq_len arg added in main, shifting positionals and dropping MAX_MM_RANGES (Triton CompilationError at 238:19). - test_int8_per_token_head_raw_cache_matches_round_reference called triton_reshape_and_cache_flash_per_token_head_quant without the required kv_quant_mode arg. Signed-off-by: JartX <sagformas@epdcenter.es>
Head branch was pushed to by a user without write access
…ect#40835) Signed-off-by: JartX <sagformas@epdcenter.es> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
…ect#40835) Signed-off-by: JartX <sagformas@epdcenter.es> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
…ect#40835) Signed-off-by: JartX <sagformas@epdcenter.es> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
…ect#40835) Signed-off-by: JartX <sagformas@epdcenter.es> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Based on PR:
#40633
Changed to only INT4_PER_TOKEN_HEAD
INT4_PER_TOKEN_HEAD