Skip to content

[Bugfix] Wrap parallel-sampling child seeds into int64 - #63572

Open
vmanvs wants to merge 1 commit into
vllm-project:mainfrom
vmanvs:fix-parallel-sampling-seed-overflow
Open

vmanvs wants to merge 1 commit into
vllm-project:mainfrom
vmanvs:fix-parallel-sampling-seed-overflow

Conversation

@vmanvs

@vmanvs vmanvs commented Oct 25, 2026 •

Copy link
Copy Markdown

Overview

Fix an EngineCore crash for parallel sampling (n>1) when the request seed is the top of the int64 range; child seeds are now wrapped into int64 instead of overflowing. Follow-up to #57018, which bounds only the parent seed.

Claims

  • SamplingParams(n=2, seed=2**63 - 1) no longer kills the EngineCore with OverflowError / EngineDeadError on Model Runner V2 (default runner).
  • No change in output for any seed that did not overflow before: child seeds are still seed + index whenever that fits in int64
  • On Model Runner V1 the wrapped seed initializes the torch.Generator identically to the old unwrapped one, so MRV1 outputs are unchanged even for the edge case.

Validation

Hardware: GTX 1650 (4 GB, sm_75), WSL2 Ubuntu, torch 2.14.0+cu132, MRV2 (default).

New LLM-level regression test, without the fix (fix stashed, tests kept):

$ python -m pytest tests/v1/engine/test_llm_engine.py::test_parallel_sampling_max_seed -v
(EngineCore) ERROR [core.py:1574]   File "vllm/v1/worker/gpu/sample/states.py", line 58, in add_request
(EngineCore) ERROR [core.py:1574] OverflowError: Python int too large to convert to C long
E   vllm.v1.engine.exceptions.EngineDeadError: EngineCore encountered an issue. ...
FAILED tests/v1/engine/test_llm_engine.py::test_parallel_sampling_max_seed
========================= 1 failed in 90.22s =========================

With the fix:

$ python -m pytest tests/v1/engine/test_llm_engine.py::test_parallel_sampling_max_seed -v
tests/v1/engine/test_llm_engine.py::test_parallel_sampling_max_seed PASSED
========================= 1 passed in 92.83s =========================

$ python -m pytest tests/v1/engine/test_parallel_sampling.py -v
... test_child_seeds_stay_in_int64[5-3-expected0] PASSED
... test_child_seeds_stay_in_int64[9223372036854775807-2-expected1] PASSED
... test_child_seeds_stay_in_int64[9223372036854775806-3-expected2] PASSED
============================== 6 passed in 3.46s ===============================

Without the fix, the two overflow cases of test_child_seeds_stay_in_int64 fail and the other 4 tests pass.

Standalone reproducer
from vllm import LLM, SamplingParams

llm = LLM("facebook/opt-125m", enforce_eager=True, max_model_len=256)
params = SamplingParams(n=2, seed=2**63 - 1, temperature=1.0, max_tokens=8)
print([o.text for o in llm.generate(["Hello, my name is"], params)[0].outputs])

Before: OverflowError at vllm/v1/worker/gpu/sample/states.py:58, then EngineDeadError.
After: two distinct completions.

MRV1 seed mapping check (torch.Generator().manual_seed(s).initial_seed()):

seed passed initial_seed
2**63 (old child seed) 9223372036854775808
-(2**63) (new child seed) 9223372036854775808

Lint: ruff check, ruff format, typos, mypy (tools/pre_commit/mypy.py 3.11) and the local Python hooks pass on the changed files.

Model evals: not run. Output is unchanged for every seed that previously worked; the only behavioral change is for seeds that previously crashed MRV2.

Details

Root Cause: ParentRequest._get_child_sampling_params derives each child's seed as seed + index on a shallow copy of the parent SamplingParams, after validation has already run. The OpenAI protocol (and #57018 for SamplingParams) accepts any seed in [-2**63, 2**63 - 1], so a parent seed near the top of that range yields child seeds above 2**63 - 1. MRV2 stores per-request seeds in an int64 UvaBackedTensor (SamplingStates.add_request), and the numpy assignment raises OverflowError inside EngineCore, killing the engine for every in-flight request.

Fix: Wrap the child seed into int64 with two's complement: (seed + index - INT_64MIN) % 2**64 + INT_64MIN. This is the identity whenever seed + index already fits, so existing seeded results are reproducible bit-for-bit, and children still get distinct seeds (n is far below 2**64). Wrapping (rather than rejecting the request) keeps every seed the API accepts usable with n>1, and matches how torch.Generator.manual_seed interprets negative seeds, so MRV1 behavior is unchanged.

Why this is not a duplicate. #57018 (open) bounds SamplingParams.seed itself; the overflow here happens to derived child seeds after that check, so it is not fixed by #57018. No open PR touches vllm/v1/engine/parallel_sampling.py. #58914 (Rust frontend n support) and #58438 (DiffusionGemma diffusion_samples) derive per-sample seeds with the same seed + index pattern and may want the same wrap; they are not changed here.

Tests. A parametrized unit test on ParentRequest pins the child-seed contract (including the identity case), and one LLM-level test in tests/v1/engine/test_llm_engine.py reproduces the crash through LLM.generate. Both run in the existing ":nvidia: (L4) V1 Engine" Buildkite step (pytest -v -s v1/engine).

Pull Request Checklist
  • I used vLLM's /pr-checklist skill. (Mandatory for agents, optional for humans).

  • AI assistance was used during the creation of this PR.

  • Design Fit: Minimizes impact on core components, reuses existing functionality, and justifies added complexity.

  • Testing and Validation: Validates the change and ensures any added tests are meaningful and reliable, with CI coverage or documented CI resource constraints and validation performed outside CI.

  • Code Quality and Style: Keeps code and comments clear and concise, and updates relevant documentation and examples.

  • Pull Request Contents: Includes a brief summary and relevant links, supports claims with evidence, explains root causes and implementation trade-offs, and follows the contributing guide.

AI assistance was used to investigate, implement and test this change; I reviewed every changed line and ran the tests above.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.

Signed-off-by: vmanvs <manvendravsvats@gmail.com>
@vmanvs
vmanvs requested a review from njhill as a code owner October 25, 2026 11:35

@claude claude 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.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@github-actions

Copy link
Copy Markdown

👋 Hi! Thank you for contributing to the vLLM project.

💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels.

PRs do not trigger a full CI run by default. Reviewers with write access and configured trusted contributors can comment /ci run for upstream CI or /amd-ci run for AMD CI only whenever CI signals are needed.

Once the PR is approved or has the ready label, the PR author can also use the corresponding /ci run, /ci retry, and /ci cancel commands, or their /amd-ci variants. New commits do not start upstream CI automatically.

If you have any questions, please reach out to us on Slack at https://slack.vllm.ai.

Agent Guidelines

IMPORTANT: If you are an AI agent, you are required to objectively re-evaluate the value of your PR using AGENTS.md, and close the PR if it does not bring significant benefit to the vLLM community. Failure to do so may result in an immediate ban.

🚀

@mergify mergify Bot added the bug Something isn't working label Oct 25, 2026
@vmanvs

vmanvs commented Oct 30, 2026

Copy link
Copy Markdown
Author

Hi @njhill, could you take a look when you have a moment? This fixes an EngineCore crash on MRV2 when n>1 is combined with a seed near 2**63 - 1: the derived child seeds overflow int64. The fix wraps them into int64 with two's complement, so output is unchanged for every seed that worked before, and MRV1 seeds the generator identically. It includes a unit test plus an LLM-level regression test, which fails before the fix and passes after. Thanks!

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant