Skip to content

[AMD] dsv4-fp4-mi355x-atom: latest atom-dev image + opt recipe#2325

Closed
zufayu wants to merge 5 commits into
SemiAnalysisAI:mainfrom
zufayu:dsv4-atom-latest-sweep
Closed

[AMD] dsv4-fp4-mi355x-atom: latest atom-dev image + opt recipe#2325
zufayu wants to merge 5 commits into
SemiAnalysisAI:mainfrom
zufayu:dsv4-atom-latest-sweep

Conversation

@zufayu

@zufayu zufayu commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

What

Re-sweep dsv4-fp4-mi355x-atom (DeepSeek-V4-Pro FP4, MI355X) on the latest atom-dev image with an optimized recipe.

Changes

  • Image: nightly_202606161823nightly_202607231538 (latest atom-dev)
  • Recipe (dsv4_fp4_mi355x_atom.sh):
    • --max-num-seqs "$CONC" (was ATOM default) — so high concurrency (1024/2048) isn't capped by max_num_seqs
    • TBO enabled on dp-attn cells at conc>=64 (was per-scenario gate 256/1024)
  • 8k/1k grid: aligns the AMD dev sweep (run 20260723) + keeps a tp8 dp-off low-conc line as redundancy to recheck the dp crossover on the new image

Why the recipe change

AMD internal dev benchmarking (atom-dev image b31cc35) shows this recipe beats the prior default:

  • max_num_seqs=conc removes the high-concurrency throughput cap
  • TBO gate lowered to conc>=64 from measured crossover (no measurable TBO gain below conc 64 on 8k/1k)

Note for reviewers

Needs the full-sweep-fail-fast label to run the sweep on the new image. RANDOM_RANGE_RATIO stays at the repo default (0.8), matching the dev runs.

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com

- image: nightly_202606161823 -> nightly_202607231538 (latest atom-dev)
- recipe (dsv4_fp4_mi355x_atom.sh): max_num_seqs=conc + TBO on dp-attn cells
  at conc>=64 (was ATOM-default max_num_seqs + TBO per-scenario gate 256/1024)
- add 1k1k scenario (was 8k1k only; mirrors prior grid)
- 8k1k grid: align AMD dev sweep (run 20260723) + tp8 dp-off low-conc redundancy

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

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

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.

@zufayu

zufayu commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator Author

@seungrokj @chunfangamd — this PR re-sweeps dsv4-fp4-mi355x-atom (DeepSeek-V4-Pro FP4, MI355X) on the latest atom-dev image (nightly_202607231538, up from nightly_202606161823) with the opt recipe validated in AMD dev benchmarking:

  • --max-num-seqs=conc — removes the high-concurrency throughput cap (1024/2048 were previously capped by the default)
  • TBO on dp-attn cells at conc>=64 (was per-scenario gate 256/1024); measured no TBO gain below conc 64 on 8k/1k
  • adds the 1k/1k scenario (was 8k/1k only)
  • 8k/1k grid aligned to the dev sweep + a tp8 dp-off low-conc line kept as redundancy to recheck the dp crossover on the new image

RANDOM_RANGE_RATIO stays at the repo default (0.8), matching the dev runs.

Could you please review and add the full-sweep-fail-fast label to kick off the sweep? Happy to adjust the grid/recipe if needed. Thanks!

max_req=conc is a mid/high-concurrency optimization. At very low conc (e.g. c2)
the ATOM default is on-par or ~4% better, so only set --max-num-seqs=conc for
conc>=64 (matches the TBO gate). High conc still gets it (removes the cap).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@zufayu

zufayu commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator Author

Correction on the recipe (pushed): max_num_seqs=conc is now gated at conc>=64, not applied to every point.

Dev finding: max_req=conc is a mid/high-concurrency optimization. At very low concurrency (e.g. c2) the ATOM default is on-par or ~4% better than max_num_seqs=2, so low conc now uses the default. High conc still gets max_num_seqs=conc (removes the throughput cap). This matches the TBO gate (conc>=64).

Comment thread configs/amd-master.yaml Outdated
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@zufayu

zufayu commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator Author

Dropped the 1k/1k scenario — this sweep is 8k/1k only now. Config keeps just the 8192/1024 grid.

@zufayu zufayu changed the title [AMD] dsv4-fp4-mi355x-atom: latest atom-dev image + opt recipe + 1k1k [AMD] dsv4-fp4-mi355x-atom: latest atom-dev image + opt recipe Jul 24, 2026
@functionstackx

Copy link
Copy Markdown
Collaborator

also would yall like to submit ATOM dsv4 towards the agentic coding scenario, seems impactful and more real world to spend time on that @chunfangamd @zufayu

dp-attention keeps a full KV pool per rank, so the large default
max_num_seqs OOMs even at low conc (c16/c32). Set max_req=conc for
every dp-on cell; dp-off low conc still uses the ATOM default.
TBO gate unchanged (conc>=64).
@zufayu
zufayu requested a review from functionstackx July 24, 2026 07:57
@zufayu

zufayu commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator Author

agentic

The agentic scenarios are run by other dedicated AMD developers who push their branches upstream — I'm on the dsv4-atom line and need the same access.

@zufayu

zufayu commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator Author

Hi @functionstackx — could you add zufayu as a collaborator? This PR is from a fork, so its sweep gets no secrets and fails at checkout (Input required and not supplied: token). I'm on the dsv4-atom line and need write access to push my branch and trigger the sweep. Thanks!

PS: Two-factor authentication Configured

@zufayu

zufayu commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator Author

Superseded by #2327 — the branch is now in upstream (amd/dsv4-fp4-mi355x-atom-latest), where the sweep can access secrets and actually runs (currently in progress). Closing this fork PR.

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

Projects

Development

Successfully merging this pull request may close these issues.

3 participants