Skip to content

feat(replay): replay DGDR trace Runs with common random numbers - #17733

Merged
tedzhouhk merged 4 commits into
hzhou/dgdr-trace-pvcfrom
hzhou/live-retune-1-crn
Nov 2, 2026
Merged

tedzhouhk merged 4 commits into
hzhou/dgdr-trace-pvcfrom
hzhou/live-retune-1-crn

Conversation

@tedzhouhk

@tedzhouhk tedzhouhk commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

Note

Stacked PR, live retune (split from #17562). Merge order: #17483 → #17733 (this PR) → #17734 → #17735 → #17736 → #17562

This PR targets hzhou/dgdr-trace-pvc, the branch of #17483 (the PR below it), so its diff shows only this layer. #17562 was split into these layers for review. At the split they carried #17562's original diff exactly, plus a 40-line replay check in #17734 that rejects executor: dgdr in offline replay; the review follow-up of 2026-11-02 has since changed several layers.

Overview

Live planner retune (#17562) compares Candidates that different DGDR Runs of one window scored. That comparison only works if a Candidate's score depends on its own configuration and nothing else. This PR makes DynamoReplayRunner replay every trace workload with common random numbers (CRN): ordinal request ids and the canonical router seed. DynamoReplayRunner is the dynamo replay runner that AISimulate's Sweeper loads (entry point in pyproject.toml), and it is what a DGDR trace Run executes.

run_trace_replay already has the internal _deterministic option for this; offline replay retune uses it for its baseline and candidates. This PR only passes it from the Sweeper runner. The synthetic-workload path is unchanged.

This layer changes no file that #17483 touches. It sits on #17483 for merge order, because DGDR trace Runs (spec.workload.trace) come from #17483.

Key Changes

  • components/src/dynamo/replay/simulation.py: passes _deterministic=True to both run_trace_replay calls of DynamoReplayRunner (agg and disagg). run_synthetic_trace_replay has no such parameter and does not receive it.
  • components/src/dynamo/replay/api.py: comments only. _deterministic is no longer described as internal to retune sweeps: every trace replay through the Sweeper's dynamo runner sets it.
  • components/src/dynamo/replay/tests/test_simulation.py: the existing test_trace_runner_preserves_current_replay_arguments now runs for an agg and a disagg deployment, checks each one's worker counts, and asserts _deterministic is True.

Behavior note: this applies to every trace replay through the Sweeper's dynamo runner, including every DGDR trace Run, not only the Planner's. A Run's scores no longer depend on the other candidates or on the Run's parallelism.

Validation

Ran on this layer (e9ab6afc24, before the review follow-up) and compared with the same suites at the stack base 6e8087d359 (#17483's head, with main c6b055d4c7 merged in):

Check Result
replay/tests/test_simulation.py::test_trace_runner_preserves_current_replay_arguments (agg, disagg) 2 passed, run at the top of the stack (92cee14c88), where simulation.py and test_simulation.py are identical to this layer's head 8e2df8215e. The rest of the file: 54 passed, and 2 failed on this host's missing conversation_affinity (see below).
Planner and replay suites (components/src/dynamo/planner/tests, components/src/dynamo/replay/tests, no xdist) At e9ab6afc24: 59 failed, 2137 passed, 2 skipped. At the base: 59 failed, 2131 passed, 2 skipped. The two sets of failing tests are identical; the 6 extra passes were the tests of the earlier separate CRN test file, which the follow-up folded into test_simulation.py. Not re-run per layer after the follow-up.
pre-commit run --files on the changed files At e9ab6afc24, on its 2 changed files: all hooks pass, no files changed. After the follow-up, pre-commit from 8d098bed48 to 92cee14c88 (the top of the stack, covering every layer's follow-up) is clean.
CI mypy (the dynamo-pipeline.yml command, MYPYPATH=components/src:lib/bindings/python/src mypy --explicit-package-bases) on dynamo/planner and dynamo/replay At e9ab6afc24: 9 errors, the same 9 as at the base: missing yaml/pandas/nvtx stubs, plus existing arg-type errors in common/http/url_validator.py and planner/simulation/provider.py. None is new.

The 59 failures come from this host. Its prebuilt bindings lack conversation_affinity (#16854) and the ais-forward-pass feature, and it has no etcd binary. CI builds the bindings from source.

The development-cluster run in #17562 used a planner image with this change for its Run Jobs, on #17562's pre-split branch. Both windows of that run reached a decision (kept, applied). The executor allows a decision only when the CRN probe's score matches the grid evaluation of the same point within a relative error of 1e-9. This layer was not run on a cluster by itself.

Issues and Ongoing Work

Review follow-up (2026-11-02)

  • Folded the determinism check into the existing test_trace_runner_preserves_current_replay_arguments, with an agg and a disagg row, and removed the separate CRN test file.
  • Cut the runner comment down to the invariant: trace replays use ordinal request ids and the canonical router seed, so a candidate's score depends only on its configuration and the trace.
  • Updated the replay/api.py comments: _deterministic now applies to every trace replay through the Sweeper's dynamo runner, not only to retune sweeps.
  • Merged feat(planner): live retune from recorded traces through DGDR v1beta2 #17483's review follow-up into this branch.
Pull Request Checklist
  • I used Dynamo's /pr-checklist skill.

🤖 Generated with Claude Code

DynamoReplayRunner now replays every candidate of a trace sweep with
_deterministic=True: ordinal request ids and the canonical router seed.
Candidates of one trace then differ only by their configuration, so a
candidate's score does not depend on the other candidates, the Run or the
parallelism. Live planner retune compares candidates that different DGDR
Runs of one window scored on that basis. The synthetic-workload path is
unchanged.

First layer of the live retune stack split from #17562.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: hongkuanz <hongkuanz@nvidia.com>
@tedzhouhk
tedzhouhk requested a review from a team as a code owner October 30, 2026 23:42
@github-actions github-actions Bot added the feat label Oct 30, 2026
tedzhouhk added a commit that referenced this pull request Oct 30, 2026
Stacks this PR on #17736. The library layers of live retune now come from
#17733 (CRN replay), #17734 (configuration and premises), #17735 (window
cut and reconciler) and #17736 (DGDR executor), so this PR's diff against
#17736 is the supervisor, state, actuator, metrics, the Planner wiring,
Helm RBAC, docs and the end-to-end tests. #17734 also adds a replay check
that rejects executor 'dgdr'.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: hongkuanz <hongkuanz@nvidia.com>
@devin-ai-integration

devin-ai-integration Bot commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor

✅ Dynamo PR CI passed — run 41524032893 (attempt 1) on ce59903e10

Gate checks: ✅ backend-status-check · ✅ deploy-status-check · ✅ dynamo-status-check

Posted automatically by Devin for run 41524032893. Updated on every full-CI run of this PR.

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

Reviewed at e9ab6af. The change is fine: the canonical seed is the same for every candidate and window, no user seed is overridden, and the offline-only guard can't trip. Two nits inline.

tedzhouhk and others added 2 commits November 2, 2026 08:26
Fold the trace replay determinism check into
test_trace_runner_preserves_current_replay_arguments with an agg and a
disagg row, and drop the separate test file. Cut the runner comment to
the invariant, and update replay/api.py: _deterministic now applies to
every trace replay through the Sweeper's Dynamo runner, not only to
retune sweeps.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: hongkuanz <hongkuanz@nvidia.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: hongkuanz <hongkuanz@nvidia.com>
@pull-request-size pull-request-size Bot added size/M and removed size/L labels Nov 2, 2026
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: hongkuanz <hongkuanz@nvidia.com>
@tedzhouhk
tedzhouhk merged commit df3a979 into hzhou/dgdr-trace-pvc Nov 2, 2026
119 of 134 checks passed
@tedzhouhk
tedzhouhk deleted the hzhou/live-retune-1-crn branch November 2, 2026 23:57
@sttts sttts mentioned this pull request Nov 5, 2026
22 of 31 tasks
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.

3 participants