Repository navigation
fix(cpu-ep): k-major output layout for non-final-axis TopK - #774
Merged
Merged
Conversation
…ase 0 NO-SHIP, CI fix) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CPU-EP TopK wrote outputs in push order [outer][inner][k]; ONNX requires k-major [outer][k][inner] (input shape with shape[axis]=k). The layouts match only when inner==1 (final axis), so non-final-axis TopK with k>1 produced wrong values+indices ordering vs CUDA EP / ONNX Runtime. Write each winner to its strided destination (outer*k + rank)*inner + i instead of pushing sequentially. Adds non-final-axis and first-axis regression tests; final-axis path unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
justinchuby
enabled auto-merge (squash)
August 11, 2026 14:54
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #774 +/- ##
==========================================
- Coverage 80.05% 79.36% -0.70%
==========================================
Files 331 331
Lines 137462 137491 +29
Branches 137462 137491 +29
==========================================
- Hits 110050 109120 -930
- Misses 23069 24031 +962
+ Partials 4343 4340 -3
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
justinchuby
added a commit
that referenced
this pull request
Aug 11, 2026
## Motivation @justinchuby noticed the CPU-EP `TopKKernel` isn't vectorized and does a **full sort** of every lane's entire axis (`O(width·log width)`) then truncates to k — even when `k << width`. Meanwhile the **sampler** (`logits.rs::top_k_threshold`) already uses an efficient bounded-heap / `select_nth_unstable_by` (introselect) approach. This aligns the kernel with that faster technique. ## Change Replace the per-lane full sort with: 1. `select_nth_unstable_by(k-1, …)` — introselect partition, **O(width)** average, moves the k winners to the front. 2. `sort_by` on just the k winners — **O(k·log k)**. For a 152k-wide vocab axis with k=40 this is ~an order of magnitude fewer comparisons. `topk_order` is a total order (ties broken by index), so the partition is deterministic; output layout (#774 k-major), index tie-breaking, and both `largest`/`smallest` orders are unchanged. ## Tests - New `topk_partial_select_breaks_ties_by_index` (k<width with duplicate 5.0s → must pick lowest indices [0,1,2]). - New `topk_smallest_partial_select` (largest=false, ascending partition). - All existing selection tests unchanged. `cargo test -p onnx-runtime-ep-cpu --lib selection`: **24 passed / 0 failed**. fmt + clippy clean. Generality: no model-name/shape special-casing. Builds on #774. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
🔴 Benchmark Regression DetectedComparison of criterion micro-benchmarks: PR head vs merge-base, measured on the same runner in the same job (base first → PR second).
Visual flags: Host infoWhat this cannot catch
|
justinchuby
added a commit
that referenced
this pull request
Aug 11, 2026
Bookkeeping: merge 37 decision-inbox drops into decisions.md (detail archived to decisions-archive/2026-08.md), add standing entries for #774 (TopK k-major fix), #775 (partial-select TopK perf), #779 (27B hybrid-GDN native enablement). No code changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
justinchuby
added a commit
that referenced
this pull request
Aug 12, 2026
…detail (#825) Justin asked to update `docs/PROGRESS.md` with the latest progress and drop stale info. It had grown to **1166 lines** of day-by-day changelog spanning 2026-07-15 → 2026-07-27, with the entire recent arc missing. ## Changes (1166 → 303 lines) - **Added recent milestones (2026-07-28 → 2026-08-12), newest-first:** - EP plugins run inside ORT via the plugin-EP C ABI (#762) and ship on PyPI — **`nxrt-ep-cpu` + `nxrt-ep-cuda` 0.1.0.dev5 both LIVE** (#819/#824/#820). - Absent-optional-output memory-safety wave (#762 review; Miri-clean). - VMM / KV-layout / offload / batching residency wave — managed no-spill VMM default, layout-governed KV floor (768× reduction), offload+capture coexist, #736 over-reservation audit (#755–#814). - Issue triage + autonomous fixes (#784/#785/#786/#788/#789); CPU-EP TopK correctness+perf (#774/#775). - Qwen3.5/3.6-27B hybrid Gated-DeltaNet native CUDA (#779); GLM-4-9B + DeepSeek-V2-Lite native + ORT 1.28 3-config fairness (#766/#770/#771). - Fused QMoE decode kernel + QMoE arc concluded; CUDA-graph capture trilogy, version-selectable CUDA (#708/#728/#757/#760), megakernel study (#769). - **Compressed stale detail:** the ~40 dated July day-by-day sections + verbose diffusion/LoRA/ORT2 narrative collapsed into a ~35-line "Foundational milestones" spine; superseded perf-wave numbers dropped; §1–§40 table condensed to a compact grid. - **Header refreshed:** last-updated → 2026-08-12; Published line now lists both EP plugins on PyPI + Miri CI. - **Preserved & updated** the DeepSeek/GLM known-gaps checklists and added new open items. Docs-only; full narrative remains in `.squad/decisions-archive/` and `docs/benchmarks/`. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@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
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.
Problem
CPU-EP
TopKKernelbuilt its values/indices outputs by sequentialpushin loop orderouter → inner → rank, i.e. memory layout[outer][inner][k]. ONNX TopK output keeps the input shape withshape[axis]=k→ correct layout is k-major[outer][k][inner]. These match only wheninner==1(final axis), so non-final-axis TopK withk>1emitted wrong values+indices ordering vs the CUDA EP / ONNX Runtime. Dtype-independent, pre-existing, latent (real router TopK is final-axis).Fix
Write each rank-
rwinner to its strided destination(outer*k + r)*inner + iinstead of pushing sequentially. Removed the now-unusedsortedfield (kernel always emits sorted winners — valid for bothsorted=0andsorted=1).Tests
topk_non_final_axis_uses_k_major_layout([2,3,2], axis=1, k=2),topk_first_axis_is_k_major([3,2], axis=0, k=2) — assert k-major values AND indices. Old code emits e.g.[5,3,4,2]vs correct[5,4,3,2].cargo test -p onnx-runtime-ep-cpu --lib selection: 22 passed / 0 failed. fmt + clippy clean.Generality: no model-name/shape special-casing.