Skip to content

[Fix] Keep speculative chain samples on positive support - #35

Open
rchalamala wants to merge 1 commit into
integration/kimi-k3-v0.5.20from
fix/spec-positive-support-cdf
Open

rchalamala wants to merge 1 commit into
integration/kimi-k3-v0.5.20from
fix/spec-positive-support-cdf

Conversation

@rchalamala

@rchalamala rchalamala commented Sep 25, 2026 •

Copy link
Copy Markdown

Motivation

The chain sampler's reduction and prefix scan can round differently near the upper CDF endpoint. A valid draw can then select a zero-mass tail or fall through to the final vocabulary token.

Modifications

Require CDF candidates to have positive mass and lie within the vocabulary, and find the first candidate with one minimum-index reduction. If no candidate matches, use the last positive token only when the target probabilities are finite and nonnegative, the sampling mass is finite and positive, and the final uniform is in [0, 1). The extra validation scan runs only on this fallback path.

Malformed distributions, zero residual mass, and invalid uniforms remain unsupported inputs; this change does not introduce an error signal for callers.

Accuracy Tests

  • On B300, the direct registered suite passes all 49 synthetic float32 eager cases and its CUDA graph test, including support moved between blocks. The restored source passes again.
  • With only the sampler source restored to the release base, nine binary endpoint cases and the graph case select zero mass. Removing only the positive-support mask produces three eager failures and the graph failure; removing the fallback's original-target validity check produces two failures. The equal-fraction fixture passes on this GPU and reproduces separately under the CPU interpreter.
  • A validation composition using the public #37134 probability guards in all three residual passes passes the same suite and 36 eager/graph q cases. The forward-only composition fails 16 endpoint checks, confirming that its reverse pass needs the identical guard.
  • The registered seeded-coin GPU neighbor passes six methods. Sampling parameter and compiled DFlash CPU neighbors pass 86 tests.
  • Pinned lint, formatting, import sorting, spelling, registered-test validation, direct runners, and whitespace checks pass.

Speed Tests and Profiling

Matched synthetic float32 B300 CUDA-graph timings use batches of 1 and 8, target/all-correct/rejection paths, dense/sparse finite probabilities, alternating baseline/treatment order, repeated samples, and baseline/baseline null controls. Across 72 ordinary cases covering both source comparisons, none exceeds the 5% regression threshold. Per-case median CUDA latency changes are:

Vocabulary This change vs release With sgl-project#37134 vs q-only baseline
4,096 -16.82% to -4.96% -15.90% to -5.14%
8,193 -9.25% to -0.08% -5.67% to +3.37%
65,536 -4.88% to -2.11% -5.38% to -2.59%

Deliberately forced endpoint fallback costs 13.33–18.93 microseconds for this change across six cases. The release takes 6.48–10.32 microseconds while returning zero-mass tokens in those cases, so that comparison is not between correct samplers. These are operator measurements, not serving latency or throughput; no model or serving benchmark is claimed.

Checklist

  • Follow the code style and formatting guidance.
  • Add a registered synthetic regression with a direct runner.
  • Document the input contract.
  • Complete bounded GPU correctness and operator timing validation.

CI States

Latest PR Test (Base): ❌ Run #36106412981
Latest PR Test (Extra): ❌ Run #36106412632
Latest PR Test (AMD ROCm 10): ❌ Run #36106412883

Mask zero-mass and padded CDF lanes. If endpoint rounding leaves a finite positive distribution without a match, select its last positive token. Keep malformed inputs outside the supported sampling contract.

Add analytic synthetic regressions for endpoint gaps, zero tails, partial blocks, uniform boundaries, tiny mass, malformed no-match inputs, and graph replay.
@rchalamala
rchalamala marked this pull request as ready for review September 25, 2026 07:28
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-25T07:30:27.659900Z b425ccc Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Devin Review

@rchalamala rchalamala changed the title [Spec] Keep final chain samples on positive support [Fix] Keep speculative chain samples on positive support Sep 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant