Repository navigation
docs: retain DistServe topology-constrained phase planning - #278
Merged
Dong1017 merged 8 commits intoOct 31, 2026
Merged
Conversation
Attribute the official OSDI 24 edition and retain the independent versus paired-stage feasible sets, joint-SLO GPU denominator and source limits. Agent-assisted-by: Codex / gpt-6.1-sol [effort=max] (author) Signed-off-by: Dong1017 <55723162+Dong1017@users.noreply.github.com>
Regenerate the native catalog from the committed canonical skill snapshot. Agent-assisted-by: Codex / gpt-6.1-sol [effort=max] (author) Signed-off-by: Dong1017 <55723162+Dong1017@users.noreply.github.com>
Keep original-archive import blocked while exposing the separately qualified phase-placement reference and its candidate/not-run limits in both matrices. Agent-assisted-by: Codex / gpt-6.1-sol [effort=max] (author) Signed-off-by: Dong1017 <55723162+Dong1017@users.noreply.github.com>
Regenerate the native catalog from the committed canonical skill snapshot. Agent-assisted-by: Codex / gpt-6.1-sol [effort=max] (author) Signed-off-by: Dong1017 <55723162+Dong1017@users.noreply.github.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Skill evaluationAutomatic execution / cumulative PR budget: Advisory: obtain missing evidenceRetained evidence availability: planned / unexecuted. No approval or merge authorization is issued.
PLANNED: execution is not authorized by knowing this hash. Selected comparisons: 1 · Estimated task calls: 2. Applicable only while these revisions and inputs match the current PR. Plan bindings, budget, frozen exam and advisory detailsUnchanged-correct is valid neutral evidence, not incremental benefit. Per-case outcomes (discovery excluded; regressions and gaps first): []Showing at most 20 of 0 pairs; the evidence link contains all outcomes, including omitted regressions/gaps. Plan SHA-256: Model/settings:
Frozen exam hashes: Provider/settings: Uncovered / unsupported paths (81)
|
Dong1017
added
the
area:knowledge
ChatGPT Codex Connector
Distinguish common outer layer grouping from complete per-phase layouts. Retain the reported unequal PP example and unqualified helper details. Agent-assisted-by: Codex / gpt-6.1-sol [effort=max] (author) Signed-off-by: Dong1017 <55723162+Dong1017@users.noreply.github.com>
Refresh both native projections after the canonical DistServe correction. Agent-assisted-by: Codex / gpt-6.1-sol [effort=max] (author) Signed-off-by: Dong1017 <55723162+Dong1017@users.noreply.github.com>
Dong1017
commented
Oct 31, 2026
Dong1017
left a comment
Dong1017
left a comment
Collaborator
Author
There was a problem hiding this comment.
LGTM for the bounded source-only candidate slice at 7ef53189; I found no blocking content or packaging issue.
- The two feasible sets match Algorithms 1–2; the shared outer grouping is kept separate from complete per-phase TP/PP layouts. Table 3 supports the unequal-PP qualification, and the simulation/label limits are retained.
- Current-head Ready checks and the latest declaration status passed. Skill Eval is planned/unexecuted; my recommendation covers attributed source guidance with
candidate / not_runpreserved, not empirical skill or model benefit. - Integration remains with the existing author, @Dong1017. After another of PRs #277–#279 lands, preserve all canonical additions, commit the combined source snapshot, regenerate the native catalog and coverage view, then verify the new head. In particular, #277/#278 share the search-policy owner; choosing one generated side would lose the combined projection.
Evidence and scope
- Reviewed head:
7ef53189343c4bf04fc9e31b4d80f9644482cdfc. - API base:
ffb77f9a168ba1cb22f7ba261f289d24dbb6698f; assessed main:3ece458ceaa92c5dad8d8e0b1e782a6961400b96. - Hosted tested merge:
a930a629d3cfab4a0d3ee83c6a0bf5b7a1a08f9c, with verified parents main + reviewed head. Ready run: Python 3.10/3.11 full-check and installed-package jobs, plus KV task qualification succeeded. The 3.10 job reports 3168 passed, 13 skipped and 284 passing subtests. Latest declaration run succeeds; earlier failed runs remain historical. Declaration compliance does not verify actual author settings. - Scope: DistServe reference/source record, owner procedure/metadata, both matrices and all generated catalog/coverage changes. Canonical source snapshot
6f8f6fbbfa100bc444ea062656efe6fc365f382eis reachable; only projections change between it and this head. Changed owner/resource hashes match the catalog; sidecar fields are preserved, with the reader's expected name/description fields added. Official paper bytes match the recorded PDF SHA-256. Source locations and historical results were checked; I did not reproduce the paper's experiments. - Skill Eval plan: 1 selected comparison(s), 91 uncovered/unsupported changes; execution skipped with unavailable/expired trusted configuration/pricing. These gaps are explicitly retained for this source-only review, with no efficacy acceptance or candidate promotion. Earlier incomplete bot reviews are not approvals.
- Integration scope is conditional on the peer versions: #277
e3db289c42d6d37374755a05f28b1107da4f73ae; #2787ef53189343c4bf04fc9e31b4d80f9644482cdfc; #279cf0b7022761fa83361f986e3b37c2031ad33fd8d. Their combination has not been tested. Main's intervening activation change does not alter these knowledge inputs; each hosted merge already includes that main revision. - This review task made no implementation changes and ran no local pytest, model, GPU or paid evaluation. The posting account is also the PR author account, so this is a COMMENTED review, not GitHub APPROVED. It grants no merge, execution or whole-tracker acceptance authority.
This was referenced Oct 31, 2026
Preserve both source contributions, combine the shared owner and matrices, and retain current-main files before native projection regeneration. Agent-assisted-by: Codex / gpt-6.1-sol [effort=max] (author) Signed-off-by: Dong1017 <55723162+Dong1017@users.noreply.github.com>
Project the combined Sarathi and DistServe owner from the committed source tree, preserving candidate state, source metadata and validation limits. Agent-assisted-by: Codex / gpt-6.1-sol [effort=max] (author) Signed-off-by: Dong1017 <55723162+Dong1017@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.
Governing Issue and delivery slice
Refs #91.
DistServe's placement algorithms search different feasible sets under different cross-node bandwidth assumptions. This PR retains one attributed reference under the existing
optimization-search-policyowner: Algorithm 1's independent phase planning versus Algorithm 2's same-node corresponding-layer grouping and general per-phase intra-node layouts under combined GPU capacity. It adds source knowledge without a runtime or generic P/D procedure.Knowledge authorship declaration
Changed artifacts and evidence
The owner links the phase-placement reference and its source record. They preserve the official OSDI 24 edition/readset, joint TTFT/request-average-TPOT attained-goodput/GPU denominator, resource costs, simulated-ablation and calibration limits, and Section 6.5's label inconsistency. All performance observations remain historical author reports or simulations.
Algorithm 2's shared outer grouping is distinguished from each phase's complete intra-node configuration. Section 4.2 keeps those configurations general; Section 6.1 and Appendix B, Table 3 report Low end-to-end OPT-66B layouts with prefill TP=4/PP=1 and decoding TP=2/PP=2. The reference therefore imposes neither equal total PP nor TP-only inner enumeration. Helper internals and concrete placement reconstruction remain unqualified.
Both matrices distinguish this candidate /
not_runreference from the unavailable original six-paper archive. The subject overlaps closed-unmerged #156; this PR uses original attributed prose from the fresh official edition, does not reopen/copy that proposal, and makes no novelty claim over unseen archive synthesis. Paper-specific redistribution/derivative rights remain UNKNOWN; raw PDF, extracted text, figures and renders are excluded.The integrated base is
55c2afd1721d9b53cff7974f86066b6d46eb63ec. Normal canonical merge M is95b1e4bac05edd101392a70920fc2295074f0848; projection commit G and final head are5d82dba1dbba227394d706aedcb06f453635dc8a. The shared owner preserves Sarathi's two canonical case files and DistServe's two P2-corrected reference files byte-for-byte, together with both procedure/matrix amendments and every metadata value. The inventory stays at 77 skills and 7 cases; owner readsets are the complete union of 6 prior readsets, 1 Sarathi record and 2 DistServe resources.M temporarily retains main's generated outputs while resolving merge conflicts. Both native generators then regenerate from actual combined M into G: all 77 catalog records cite M, and coverage records the new input digest and 9 owner readsets. Earlier contribution commits and current-main files remain in the normal merge history.
Checks and dependencies
--checkmodes andgit diff --check.-I -Sand explicit existing dependency paths. Earlier broader local tests and wheel builds timed out; no local installed-package result is claimed, and those broad jobs were not repeated.Remaining work and Issue closure
Merge readiness requires fresh review and all applicable checks at the final head. This contribution does not recover the archive, establish current-engine compatibility or complete the broader #91 queue. Goal acceptance and Issue closure stay with the human owner.