[AMD] add more configs for MI355X DSR1 FP8 Disagg - #700
Merged
Conversation
billishyahao
force-pushed
the
amd/mi355x-dsfp8-feb14
branch
from
February 13, 2026 19:10
2853439 to
c52db03
Compare
Collaborator
|
@claude Please update perf-changelog.yml entry to be a bit more indicative of what actually changed performance wise. Ie the serving configurations for what scenarios etc. |
Collaborator
|
Claude finished @cquil11's task in 57s —— View job Updated perf-changelog.yaml with detailed descriptions
Updated the
Also added |
cquil11
approved these changes
Feb 14, 2026
…tions Add specifics about the serving configuration changes: Pure TP prefill switch, new 2P1D 8k1k configs, extended concurrency range, and recipe commit bump for bug fixes. Co-authored-by: Cameron Quilici <cquil11@users.noreply.github.com>
billishyahao
force-pushed
the
amd/mi355x-dsfp8-feb14
branch
from
February 14, 2026 14:05
b8f3b78 to
99f8485
Compare
Oseltamivir
added a commit
that referenced
this pull request
Aug 5, 2026
DeepEP's low-latency combine corrupts the 256 tokens/rank rung on every Blackwell SKU we run -- B200, GB200 and GB300, EP8 and EP16, both precisions, MNNVL and RDMA alike -- while Hopper stays clean. It is stochastic at roughly 1.5-3.3% per invocation and surfaces as one wrong token row whose norm still matches, so the correctness gate catches it as a 0.07-6.6 relative error against a 0.03125 tolerance. Tracked upstream as DeepEP issue #700. Clamp the measured low-latency ladder to 128 and leave the receive sized at 256. The two are now separate constants on purpose: the receive footprint drives the transport's memory traffic and the FP8 dequant volume, so sizing it from max(ladder) -- as it was -- would have halved it the moment the ladder moved and shifted every retained rung out of comparability with the published series. Holding it at 256 also leaves the top measured rung at half occupancy, which is the ladder/capacity decoupling the original capacity probe had to hand-roll. The clamp is not silent: the harness already reports every dropped ladder point into the artifact. The likely upstream fix is DeepEP PR #642, which adds a CTA-scope fence so the combine consumer's shared-memory reads retire before the stage is recycled and the producer's next TMA load refills it -- that mechanism predicts the observed signature, and it closed #621, the same race reached from NVL72. Our pin is the head of PR #605 and was branched before #642 merged, so the fence is simply absent from our build. Raising the ladder back to 256 is therefore gated on a pin bump, deliberately not bundled here: it spans months of upstream change, re-baselines every deepep-v2 row including normal mode, and needs rewrite_deepep_v2 made tolerant first, since main already carries the 'libnccl' fix that rewrite asserts it must apply.
Oseltamivir
added a commit
that referenced
this pull request
Aug 7, 2026
…ing more than it proved From an external review of the PR (queries.md items #2 and #8). The review was right, and this is the most substantive thing it found. benchmark_chain discarded every combine result, so the 'chained regime' gate was a FRESH dispatch and combine run after the chain finished. That proves the communicator still works afterwards; it does not prove the chain's own outputs were correct. The distinction is not academic here -- this project has already met invocation-dependent corruption that a later clean call would have hidden (the Blackwell low-latency top-rung defect, DeepEP #700), which is exactly the failure the old field's name implied it caught. benchmark_chain now returns the period chain's final combined output, cloned after the closing synchronize. The only change inside the timed loop is binding an existing call's return value, so the measurement is untouched. Pass 2b compares that output against a freshly drained run_roundtrip over the same problem, outside every timed region. Comparison is tolerance-based, not bit-equality: combine kernels are not order-deterministic, and no analytic model exists for the plain combine. Both verdicts are now separate fields, ANDed into passed: correctness.chain_last_output_passed -- the chain's own output was right correctness.post_chain_state_passed -- a fresh operation still passes afterwards The second is the old chain_regime_passed under an honest name. Stated plainly in code and docs: this validates the LAST pair, not all 128. Validating interior pairs means device work or ~15 GB of retained output inside the timed loop, which would corrupt the measurement the chain exists to produce. A corruption in pair 37 that later pairs overwrite is still invisible, and the docs now say so instead of implying otherwise. Item #8: both 'impossible' states are genuinely dead for new artifacts -- the budget gate rejects non-positive chain counts, and removing chain_barrier killed the only other producer of a null oracle. The single-kept-pair branch was reachable only by hand-setting chain_drop = chain_iters-1, never by the scheduler. The gate now requires two kept pairs, Pass 3 asserts the oracle ran, and the unavailable-health branch is gone. Consumer note: this RENAMES an artifact field. Verified no consumer reads it -- summarize.py keys on pair_period, and the frontend reader consumes outcome reasons rather than correctness subfields. Artifacts already in the durable store keep chain_regime_passed under the old name. 160 tests green.
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
3 participants
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.
Add more configs for MI355X DSR1 FP8 disagg. Bump recipe commit to 953f7c512b713683ac3a7182f78f393dcdf3768c for fixing bugs.
See downstream changes here: billishyahao/sglang_disagg@21c60a1...953f7c5