[pull] main from huggingface:main - #41
Open
pull[bot] wants to merge 459 commits into
Open
Conversation
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Join our Discord community for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
[Metal] Ensure tensors are send/sync
Fix broken slice_scatter example in basics.rs
Update cudarc to v0.17.3 which has support for CUDA 13.
fix kv cache issue with quantized_phi3 implementation
ug 0.5.0 pulls in cudarc 0.17.3, which supports CUDA 13 Signed-off-by: Graham King <grahamk@nvidia.com>
[Metal] various minor improvements
* is_multiple_of clippy fixes * candle-core clippy fixes * candle-nn clippy fixes * candle-transformers clippy fixes * candle-wasm-examples clippy fixes * Fix quantized avx clippy * Fix candle-transformers clippy
[Metal] Add conv ops for more dtypes
…r-map [Metal] Improve command buffer map
bump gemm dependency to 0.18.2 to match ug
* Improve metal buffer usage. Add gather u8 kernels * Use same metal resource options for all buffers. Fix copy call in allocate_buffer_with_data * Make allocate_buffer pub. remove new_buffer_managed * Remove allocate_buffer_with_data for now (needs sync)
…#3097) * Add direct copy for floats in quantization * Calling q matmul directly gives slightly different results, but within ggml error leniency * fix quantized_mm test. Flip a and b input to matmul * Add compile time verification of block sizes being equal to vec dot type block sizes * Since we have verified that the block sizes are equal we can simplify qmatmul * clippy * Improved direct copy. Add comment to debug assert * Add more info to quantized matmul test failures * Disable quantized_mm for bf16 temporarily
* add dot.rs; rm 4 dupe dot impl; migrate dot_32 call site; lean dispatch standard cpu_flash * softmax helper; impl in 13 locations; dot error compound bug resolved * drop f64 path from qwen3 dispatch * rm 2 line internal comment * fmt standard and dot * remove dead sum; 12 generic calls switched to D-type native conversion * fmt * attention score keeps full f32 range/precision * no exclusion of f64 so have bail
…around entire fwd pass instead to access the shared threadpool (#3634)
* Parameter cache for CUDA kernel launch * Enforce strict h2d/d2h invariants during CUDA graph capture * Revise guard drop * Remove clear_cuda_param_cache and d2h blocker * Add capture check for all memcpy actions * Unified guard
* Support embedding forward pass for ggml quants * Add cuda
* Fix BlockQ8K::from_float iscale/precision * Add neon dotprod optimization utils and optimized BlockQ8_0::vec_dot_4 path with neon vec_dot_4_q8_0_q8_0 * Optimized BlockQ4K::vec_dot_4 path with neon vec_dot_4_q4k_q8k * Optimized BlockQ6K::vec_dot_4 path with neon vec_dot_4_q6k_q8k * Add interleaved/repacked matmul_q4k_x8 with neon optimized path * Slightly improved neon quantize_row_q8k * Add matmul_q4k_x8 path to QTensor matmul call * No need to unsqueeze and transpose before reshape * Have candle barrier pool respond to different env var than rayon * Use candle threadpool in cpu causal FA. Other causal FA improvements * Add serial path to cpu rms norm * Add serial path to cpu rope * Update quantized test. Crossed fingers non-neon targets were affected identically * clippy * Fix pack_to_q4kx8 alignment. Return BlockQ4Kx8. Add vec_to_bytes util. * Update quantize_q8k test. * Make vec_to_bytes copy data to ensure no UB * Standard vec initialization in pack_to_q4kx8 * Use zerocopy to ensure correct alignment (already a transitive dep) * Remove unused byte/vec conversion fns
* Fix clippy warnings for Rust 1.97 `cargo clippy --fix` for lints promoted to errors on stable 1.97 (useless_borrows_in_formatting, for_kv_map, manual_filter). * Preserve NaN handling in LogitsProcessor::new f64 is only PartialOrd, so clippy's manual_filter suggestion drops NaN where the original comparison kept it. Keep the original expression and allow the lint instead.
Co-authored-by: ivarflakstad <69173633+ivarflakstad@users.noreply.github.com>
…3771) * feat(metal): register external buffers in the device residency set * Apply suggestions from code review * Address PR review --------- Co-authored-by: ivarflakstad <69173633+ivarflakstad@users.noreply.github.com>
conv2d_im2col_gemm copies a non-contiguous kernel into a fresh contiguous buffer that starts at index 0, but it built the matmul layout using the original strided kernel's start offset. For a kernel with a non-zero start offset, such as a channel slice of a larger pointwise weight, the matmul read past the materialized buffer and produced incorrect forward values and gradients. Use offset 0 so the layout matches the materialized buffer. Co-authored-by: ivarflakstad <69173633+ivarflakstad@users.noreply.github.com>
* fix(qwen3): build causal mask batch-independently (#3582) `Model::causal_mask` built the additive mask buffer with `tgt*(tgt+offset)` elements (independent of the batch) but shaped it `(b, 1, tgt, tgt+offset)`. For `b > 1` the tensor claims b× the elements actually present, so every batch row but the first reads past the buffer and is masked incorrectly, producing wrong output for batched forwards (the bug disappears at b=1). This is hit on the standard matmul attention path (e.g. Metal), as reported in #3582. Extract the mask construction into a `build_causal_mask` free function that shapes the mask `(1, 1, tgt, tgt+offset)` and relies on the existing `broadcast_add` to apply it across the batch. Add a CPU regression test asserting the mask is batch-independent and broadcasts to a causal, per-row-identical mask. The same `(b, 1, tgt, ...)` pattern exists in several sibling models (qwen3_moe, quantized_qwen3{,_moe}, glm4_new, quantized_glm4, smollm3, z_image/text_encoder); happy to fix those in this PR or a follow-up. * fix(qwen3): use usize arithmetic for sliding-window mask check Address review feedback on #3586: the sliding-window check computed `(i + offset) as i64 - j as i64 <= w as i64`, casting to signed to allow a negative result when `j > i + offset`. Rearranged to `j + w >= i + offset` — equivalent, but stays in `usize` with no signed casts and no subtraction underflow. Add a sliding-window regression test; the prior tests only covered the no-window path. * docs(qwen3): trim causal mask comments per review
Co-authored-by: ivarflakstad <69173633+ivarflakstad@users.noreply.github.com>
…pendently (#3879) Follow-up to #3586 / #3582, extending that fix to every remaining model with the same pattern and moving the shared logic into `crate::utils` alongside `build_causal_mask`, as #3437 did for the rectangular mask. Eight models filled the additive mask buffer with `tgt * (tgt + offset)` elements (independent of the batch) but shaped the tensor `(b, 1, tgt, tgt + offset)`. `Tensor::from_slice` does not validate the element count against the shape, since the blanket `impl<S: Into<Shape>> ShapeWithOneHole` discards `el_count`, so the oversized tensor is built silently and only misbehaves at use: - CPU: panics with `range end index N out of range for slice of length M` once the mask is read. - Metal/CUDA: reads past the buffer, so every batch row after the first is masked with garbage and the model returns wrong output. That is what #3582 reported. Affected: qwen3_moe, quantized_qwen3, quantized_qwen3_moe, glm4_new, quantized_glm4, smol/smollm3, smol/quantized_smollm3 and z_image/text_encoder. Unlike qwen3, where the mask path was gated to CPU-only under the `flash-attn` feature, these gate only on `l == 1`, so the broken mask was built on every multi-token forward on every backend. `utils::build_additive_causal_mask` returns `(1, 1, tgt, tgt + offset)` and is broadcast over the batch by the existing `broadcast_add`. All nine models now call it, including qwen3, which drops the per-model copy added in #3586. The sliding-window check uses `usize` arithmetic (`j + w >= i + offset`) as in #3586, avoiding the signed casts. Tests move with the helper: mask shape and batch broadcast, the KV-cache offset case, and the sliding window.
Co-authored-by: ivarflakstad <69173633+ivarflakstad@users.noreply.github.com>
Co-authored-by: ivarflakstad <69173633+ivarflakstad@users.noreply.github.com>
UgIOp1::metal_fwd calls set_output_buffer and dispatch_threads directly on the CommandsGuard, which only forwards pipeline and label calls since the concurrent-dispatch rework, so --features ug,metal has not compiled since 0.11. Deref to the underlying ComputeCommandEncoder, matching how the other metal call sites use the guard. Co-authored-by: ivarflakstad <69173633+ivarflakstad@users.noreply.github.com>
* metal: fix out-of-bounds indexing in rank-5+ strided reduce kernels * Add test --------- Co-authored-by: ivarflakstad <69173633+ivarflakstad@users.noreply.github.com>
…#3862) * fix(metal): surface command buffer errors that occur during execution Re-check the status after waiting and report it. --------- Co-authored-by: ivarflakstad <69173633+ivarflakstad@users.noreply.github.com>
…ng (#3892) * fix(flash-attn-v3): allocate the tile count semaphore The causal/local/split path selects DynamicPersistentTileScheduler, which atomicAdds params.tile_count_semaphore. run_mha_v3 memsets the params and never set it, so the kernel dereferenced a null pointer. * fix(flash-attn-v3): launch on the caller's stream run_mha_v3 hardcoded the legacy default stream. candle allocates its streams with cudaStreamNonBlocking, which do not synchronize with it, so the kernel could race the surrounding ops. Matches candle-flash-attn. * fix(flash-attn-v3): size softmax_lse to b*h*seqlen_q The buffer was over-allocated 128x. get_lse_gmem_layout is (b, h, m) and the epilogue bounds its stores by actual_seq_len, so the extra factor was never written. At 16k tokens this reclaims ~3 GB per forward. * fix(flash-attn-v3): stop clobbering the varlen window sizes FlashAttnVarLen overwrote window_size_left and window_size_right with max_seqlen_k whenever they were smaller, so the -1/0 sentinels never survived and is_causal was always 0. Every varlen call ran bidirectional, silently corrupting output for causal models, and sliding windows were widened to the full sequence the same way. * fix(flash-attn-v3): keep varlen off the GQA-packed launchers run_mha_fwd_gqa_ hardcodes FixedSeqLenTraits for every head dim, with no SEQLEN_SWITCH, so it ignores cu_seqlens and indexes a packed batch by seqlen_q and the batch stride. With use_gqa_packing set, varlen therefore read the wrong rows for every sequence past the first.
A 0 in the target shape means copy the corresponding input dimension by default, and a literal zero-length dimension when allowzero=1. The attribute was never read, so the copy behaviour was always used. Separately, the product that -1 is inferred against skipped zeros, but a copied dimension is part of the volume, so -1 came out too large. ONNX's own zero_and_negative_dim case reaches this without any attribute set. Resolve every non -1 dimension first so the inference divides by the true product, reject more than one -1, and reject -1 inference when the rest of the volume is zero rather than dividing by zero. Fixes #3907
* Use as_chunks over chunks_exact * clippy
Fixes #3893. Co-authored-by: ivarflakstad <69173633+ivarflakstad@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )