Skip to content

Add VarlenNGramHashMapping for packed/varlen batching support in DeepSeek Engram - #32358

Open
kunal-vaishnavi with Copilot wants to merge 52 commits into
mainfrom
copilot/update-ngramhashmapping-for-ogaengine
Open

kunal-vaishnavi with Copilot wants to merge 52 commits into
mainfrom
copilot/update-ngramhashmapping-for-ogaengine

Conversation

Copilot AI commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Description

NGramHashMapping operates on a dense (batch_size, sequence_length) layout and mixes n-gram hash inputs across positions within a row. Packing multiple sequences into one flat buffer (as required by ragged/packed batching engines like OgaEngine) and reshaping to (1, num_tokens) would silently leak hash-mix inputs across sequence boundaries at every packed request boundary.

This adds a new com.microsoft op, VarlenNGramHashMapping, modeled on VarlenCausalConvWithState, which accepts a cumulative_sequence_length (shape (batch_size + 1), device-resident int32) offsets tensor and clamps the n-gram window — and past_ids/present_ids history — at each packed request's own boundary instead of the flat buffer's.

  • Schema: New op in bert_defs.cc (registered in ms_opset.h) taking token-major packed input_ids ((total_tokens)) plus cumulative_sequence_length; past_ids/present_ids are now indexed per-request instead of per fixed-stride batch row.
  • CPU kernel: Parallelizes over requests via ThreadPool::TryParallelFor; validates offsets host-side.
  • CUDA kernel: Block-per-request design (mirrors VarlenCausalConvKernel); each block validates its own offsets before touching memory.
  • WebGPU kernel: Workgroup-per-request dispatch for hash ids, with a separate program for present_ids.
  • Prerequisite port: NGramHashMapping/EngramGate (from unmerged PR Add DeepSeek Engram contrib ops (EngramGate, NGramHashMapping) #32268) were not yet on this branch; selectively ported only the NGramHashMapping-related CPU/CUDA/WebGPU kernels, engram_helper files, and tests needed here (EngramGate and unrelated CausalConvWithState dilation/channels_last changes are out of scope).
  • Docs: Hand-updated docs/ContribOperators.md and docs/OperatorKernels.md to match gen_doc output.
  • Tests: Added coverage in engram_ops_test.cc for packed-vs-per-sequence equivalence, sequence-boundary no-leakage, packed chunked/decode equivalence across concurrent requests, negative ids, and invalid vocab_sizes.
# Equivalent results, but VarlenNGramHashMapping never reads across the sequence boundary
NGramHashMapping(seq_a) + NGramHashMapping(seq_b)
  == VarlenNGramHashMapping(concat(seq_a, seq_b), cumulative_sequence_length=[0, len(seq_a), len(seq_a)+len(seq_b)])

Motivation and Context

NGramHashMapping's dense (batch_size, sequence_length) layout is incompatible with OgaEngine's packed/ragged batching model (continuous batching / paged attention), where tokens from multiple in-flight requests are concatenated into a single flat, unpadded buffer. Without this change, models using DeepSeek Engram would silently corrupt n-gram hash ids at every sequence boundary whenever more than one request is batched together.

Copilot AI and others added 27 commits August 25, 2026 20:18
Co-authored-by: kunal-vaishnavi <115581922+kunal-vaishnavi@users.noreply.github.com>
Co-authored-by: kunal-vaishnavi <115581922+kunal-vaishnavi@users.noreply.github.com>
Co-authored-by: kunal-vaishnavi <115581922+kunal-vaishnavi@users.noreply.github.com>
Co-authored-by: kunal-vaishnavi <115581922+kunal-vaishnavi@users.noreply.github.com>
Co-authored-by: kunal-vaishnavi <115581922+kunal-vaishnavi@users.noreply.github.com>
Co-authored-by: kunal-vaishnavi <115581922+kunal-vaishnavi@users.noreply.github.com>
Co-authored-by: kunal-vaishnavi <115581922+kunal-vaishnavi@users.noreply.github.com>
Co-authored-by: kunal-vaishnavi <115581922+kunal-vaishnavi@users.noreply.github.com>
Co-authored-by: kunal-vaishnavi <115581922+kunal-vaishnavi@users.noreply.github.com>
Co-authored-by: kunal-vaishnavi <115581922+kunal-vaishnavi@users.noreply.github.com>
Co-authored-by: kunal-vaishnavi <115581922+kunal-vaishnavi@users.noreply.github.com>
…e for NGramHashMapping/ShortConv (schema, CPU, CUDA)

Co-authored-by: kunal-vaishnavi <115581922+kunal-vaishnavi@users.noreply.github.com>
…op docs

Co-authored-by: kunal-vaishnavi <115581922+kunal-vaishnavi@users.noreply.github.com>
Co-authored-by: kunal-vaishnavi <115581922+kunal-vaishnavi@users.noreply.github.com>
Co-authored-by: kunal-vaishnavi <115581922+kunal-vaishnavi@users.noreply.github.com>
…ipliers check, fused CUDA reduction

Co-authored-by: kunal-vaishnavi <115581922+kunal-vaishnavi@users.noreply.github.com>
Co-authored-by: kunal-vaishnavi <115581922+kunal-vaishnavi@users.noreply.github.com>
T1: templatize the chunked NGramHashMapping equivalence test over int32/int64
so the WebGPU past_ids/present_ids state shaders get execution coverage.

Q1: vectorize EngramGateScalarProgram, EngramGateProgram, ShortConvInvRmsProgram
and ShortConvPresentStateProgram over the contiguous hidden dimension using
GetMaxComponents + Flatten bindings + SumVector reductions. ShortConvProgram
stays scalar because weight is laid out (channel, 1, kernel_size).

Co-authored-by: kunal-vaishnavi <115581922+kunal-vaishnavi@users.noreply.github.com>
… CUDA grid/shared-mem, WebGPU empty input guard, negative-id tests

- Reject non-positive vocab_sizes on CPU and document the requirement in the schema
- Explain in the ShortConv doc why it is separate from CausalConvWithState
- CPU ShortConv: allocate inv_rms via GetTempSpaceAllocator; hoist row base pointers
- CUDA: raise GridSize cap from 65535 to the real 2^31-1 grid.x limit
- CUDA: stage multipliers/vocab_sizes in shared memory, mark read-only pointers __restrict__
- WebGPU: do not bind zero-sized input_ids when sequence_length == 0
- Tests: negative ids/pad_id coverage for PositiveMod and WrappedMultiply, plus vocab_sizes rejection

Co-authored-by: kunal-vaishnavi <115581922+kunal-vaishnavi@users.noreply.github.com>
Co-authored-by: kunal-vaishnavi <115581922+kunal-vaishnavi@users.noreply.github.com>
…traints

Co-authored-by: kunal-vaishnavi <115581922+kunal-vaishnavi@users.noreply.github.com>
Co-authored-by: kunal-vaishnavi <115581922+kunal-vaishnavi@users.noreply.github.com>
…s and docs

Co-authored-by: kunal-vaishnavi <115581922+kunal-vaishnavi@users.noreply.github.com>
Agent-Logs-Url: https://github.com/microsoft/onnxruntime/sessions/780c3ba6-6eaf-48ed-9080-88cac877e87c

Co-authored-by: kunal-vaishnavi <115581922+kunal-vaishnavi@users.noreply.github.com>
Co-authored-by: kunal-vaishnavi <115581922+kunal-vaishnavi@users.noreply.github.com>
Co-authored-by: kunal-vaishnavi <115581922+kunal-vaishnavi@users.noreply.github.com>
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

@tianleiwu Tianlei Wu (tianleiwu) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The packed-boundary design and the follow-up offset-safety fixes look solid, and the multi-EP tests are strong. I found one blocking WebGPU bug when the optional present_ids output is omitted, plus one shape-inference validation gap for the same one-output form. Details are inline.

Comment thread onnxruntime/contrib_ops/webgpu/bert/varlen_ngram_hash_mapping.cc Outdated
Comment thread onnxruntime/core/graph/contrib_ops/bert_defs.cc Outdated
Co-authored-by: kunal-vaishnavi <115581922+kunal-vaishnavi@users.noreply.github.com>
Copilot AI and others added 2 commits September 8, 2026 03:57
…ashmapping-for-ogaengine

Co-authored-by: kunal-vaishnavi <115581922+kunal-vaishnavi@users.noreply.github.com>
Co-authored-by: kunal-vaishnavi <115581922+kunal-vaishnavi@users.noreply.github.com>
…ashmapping-for-ogaengine

Co-authored-by: kunal-vaishnavi <115581922+kunal-vaishnavi@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Shape validation incorrectly depends on an optional output, and long prefills are serialized across all three implementations.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 18/18 changed files
  • Comments generated: 4
  • Review effort level: Balanced

Comment thread onnxruntime/contrib_ops/cpu/bert/varlen_ngram_hash_mapping.cc Outdated
Comment thread onnxruntime/contrib_ops/cuda/bert/varlen_ngram_hash_mapping_impl.cu Outdated
Comment thread onnxruntime/contrib_ops/webgpu/bert/varlen_ngram_hash_mapping.cc Outdated
Comment thread onnxruntime/core/graph/contrib_ops/bert_defs.cc
Copilot AI and others added 2 commits September 10, 2026 23:31
…ashmapping-for-ogaengine

Co-authored-by: kunal-vaishnavi <115581922+kunal-vaishnavi@users.noreply.github.com>
Co-authored-by: kunal-vaishnavi <115581922+kunal-vaishnavi@users.noreply.github.com>
Copilot AI and others added 2 commits September 11, 2026 00:07
…ashmapping-for-ogaengine

Co-authored-by: kunal-vaishnavi <115581922+kunal-vaishnavi@users.noreply.github.com>
Co-authored-by: kunal-vaishnavi <115581922+kunal-vaishnavi@users.noreply.github.com>
Comment thread onnxruntime/contrib_ops/cuda/bert/varlen_ngram_hash_mapping_impl.cu Outdated
Copilot AI and others added 2 commits September 18, 2026 08:35
Co-authored-by: kunal-vaishnavi <115581922+kunal-vaishnavi@users.noreply.github.com>
Co-authored-by: kunal-vaishnavi <115581922+kunal-vaishnavi@users.noreply.github.com>
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.

7 participants