Skip to content

Update turbo flydsl sparse attn#898

Open
wenxie-amd wants to merge 5 commits into
mainfrom
dev/wenx/update_flydsl_attn
Open

Update turbo flydsl sparse attn#898
wenxie-amd wants to merge 5 commits into
mainfrom
dev/wenx/update_flydsl_attn

Conversation

@wenxie-amd

Copy link
Copy Markdown
Collaborator

No description provided.

wenxie-amd and others added 3 commits July 21, 2026 06:39
…26a32

Co-authored-by: Cursor <cursoragent@cursor.com>
…mary)

Co-authored-by: Cursor <cursoragent@cursor.com>
Lets callers pin the run to specific good nodes (empty still lets the
scheduler allocate), useful to work around bad nodes causing JobLaunchFailure.

Co-authored-by: Cursor <cursoragent@cursor.com>
Copilot AI review requested due to automatic review settings July 21, 2026 06:45

Copilot AI 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.

Pull request overview

Updates the DeepSeek-V4 Flash run wrapper defaults and bumps the Primus-Turbo commit used by CI, with the intent of picking up newer flydsl sparse attention support.

Changes:

  • Adjust DeepSeek-V4 Flash SLURM env defaults (honor incoming node list; tweak NCCL_DEBUG; broaden NNODES condition).
  • Update experiment naming to include node count for easier run identification.
  • Bump PRIMUS_TURBO_COMMIT in CI, but also comment out the torch/jax unit-test + coverage jobs.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
examples/deepseek-v4/run_deepseek_v4_flash.sh Updates SLURM/NCCL defaults and run naming for DeepSeek-V4 Flash launcher wrapper.
.github/workflows/ci.yaml Updates the Primus-Turbo pin, but disables unit-test/coverage jobs by commenting them out.

Comment thread examples/deepseek-v4/run_deepseek_v4_flash.sh Outdated
Comment on lines +37 to 39
if [ "$NNODES" -ge 8 ]; then
export PRIMUS_TP=${PRIMUS_TP:-1}
export PRIMUS_PP=${PRIMUS_PP:-8}
Comment thread .github/workflows/ci.yaml Outdated
Comment on lines +203 to +206
# run-unittest-torch:
# permissions:
# contents: read
# actions: read # let "Write runtime summary" list this job's own steps for auto-discovered timing
The Primus-Turbo flydsl sparse-MLA attention moved from
`primus_turbo.flydsl.attention.kernels.sparse_mla_v2`
(`sparse_mla_{fwd,bwd}_v4_flydsl`) to the flat
`primus_turbo.flydsl.attention.sparse_mla_{fwd,bwd}`
(`sparse_mla_{fwd,bwd}_flydsl`); the call signatures are unchanged.

Bind the in-tree `turbo` V4 attention backend to whichever layout is
installed (new first, fall back to old), and make the turbo backend unit
test skip-guard accept either layout.

Verified against Primus-Turbo edc8d2c on MI355X/gfx950:
bench_v4_attention.py and the DeepSeek-V4 flash proxy
(USE_V4_ATTENTION_BACKEND/USE_V4_CSA_ATTENTION_BACKEND=turbo) both run.

Co-authored-by: Cursor <cursoragent@cursor.com>
Copilot AI review requested due to automatic review settings July 21, 2026 08:56
# The upstream module layout changed across releases; accept either (new flat
# ``sparse_mla_fwd`` module, or the old ``kernels.sparse_mla_v2`` package).
try:
import primus_turbo.flydsl.attention.sparse_mla_fwd # noqa: F401

Copilot AI 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.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

.github/workflows/ci.yaml:212

  • This change comments out the entire run-unittest-torch, run-unittest-jax, and coverage-summary jobs, effectively disabling unit/E2E testing and coverage reporting for the workflow. That is a major behavior change and doesn’t align with the PR’s stated scope (updating the turbo FlyDSL sparse attention). Please restore these jobs (or move them to a replacement workflow) so CI continues to execute tests/coverage.
#  run-unittest-torch:
#    permissions:
#      contents: read
#      actions: read # let "Write runtime summary" list this job's own steps for auto-discovered timing
#    env:
#      PRIMUS_WORKDIR: /mnt/apps_proxy/tas/0_public/primus_ci/actions-runner-torch
#      # PRIMUS_WORKDIR: /wekafs/primus-data/primus_safe_ci/torch
#      PRIMUS_TURBO_ATTN_V3_ATOMIC_FP32: 1
#    needs: [code-lint]
#    if: ${{ github.event_name != 'pull_request' || !github.event.pull_request.draft }}

Comment on lines +40 to +55
try:
# New Primus-Turbo layout: flat modules, function names without the _v4 infix.
from primus_turbo.flydsl.attention.sparse_mla_bwd import (
sparse_mla_bwd_flydsl as _sparse_mla_bwd,
)
from primus_turbo.flydsl.attention.sparse_mla_fwd import (
sparse_mla_fwd_flydsl as _sparse_mla_fwd,
)
except ImportError:
# Older Primus-Turbo layout: kernels.sparse_mla_v2 with the _v4 infix.
from primus_turbo.flydsl.attention.kernels.sparse_mla_v2 import (
sparse_mla_bwd_v4_flydsl as _sparse_mla_bwd,
)
from primus_turbo.flydsl.attention.kernels.sparse_mla_v2 import (
sparse_mla_fwd_v4_flydsl as _sparse_mla_fwd,
)
Comment on lines +48 to +54
try:
import primus_turbo.flydsl.attention.sparse_mla_fwd # noqa: F401
except ImportError:
pytest.importorskip(
"primus_turbo.flydsl.attention.kernels.sparse_mla_v2",
reason="installed primus_turbo has no flydsl sparse-MLA attention (turbo backend)",
)
…BUG override

- ci.yaml: uncomment the run-unittest-torch / run-unittest-jax /
  coverage-summary jobs so unit tests run again.
- ci.yaml: correct the PRIMUS_TURBO_COMMIT inline comment to the actual
  edc8d2c commit subject ("feat: add quantized tensor support for
  blockwise grouped gemm (#422)") instead of the stale branch note.
- run_deepseek_v4_flash.sh: set NCCL_DEBUG="${NCCL_DEBUG:-}" so an
  externally-provided NCCL_DEBUG is honored instead of forced empty.

Co-authored-by: Cursor <cursoragent@cursor.com>
Copilot AI review requested due to automatic review settings July 21, 2026 09:54

Copilot AI 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.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (3)

tests/unit_tests/megatron/transformer/deepseek_v4/test_v4_turbo_flydsl_attention.py:54

  • The skip-logic only probes the new forward module (and falls back on any ImportError), but the turbo adapter requires both fwd and bwd. This can make the test fail later with a confusing ImportError if only one of the flat modules is present, and it can also silently fall back to the old layout when the new module exists but fails to import due to a missing internal dependency.
# The upstream module layout changed across releases; accept either (new flat
# ``sparse_mla_fwd`` module, or the old ``kernels.sparse_mla_v2`` package).
try:
    import primus_turbo.flydsl.attention.sparse_mla_fwd  # noqa: F401
except ImportError:
    pytest.importorskip(
        "primus_turbo.flydsl.attention.kernels.sparse_mla_v2",
        reason="installed primus_turbo has no flydsl sparse-MLA attention (turbo backend)",
    )

primus/backends/megatron/core/transformer/v4_attention_kernels/_turbo_flydsl/init.py:55

  • The broad except ImportError will also catch import failures originating from missing internal dependencies of the new Primus-Turbo modules (e.g., a missing shared library), and then silently fall back to the old layout. That can hide a broken installation and make failures harder to diagnose. Prefer falling back only when the new modules or symbols are absent, and re-raise unexpected import errors.
try:
    # New Primus-Turbo layout: flat modules, function names without the _v4 infix.
    from primus_turbo.flydsl.attention.sparse_mla_bwd import (
        sparse_mla_bwd_flydsl as _sparse_mla_bwd,
    )
    from primus_turbo.flydsl.attention.sparse_mla_fwd import (
        sparse_mla_fwd_flydsl as _sparse_mla_fwd,
    )
except ImportError:
    # Older Primus-Turbo layout: kernels.sparse_mla_v2 with the _v4 infix.
    from primus_turbo.flydsl.attention.kernels.sparse_mla_v2 import (
        sparse_mla_bwd_v4_flydsl as _sparse_mla_bwd,
    )
    from primus_turbo.flydsl.attention.kernels.sparse_mla_v2 import (
        sparse_mla_fwd_v4_flydsl as _sparse_mla_fwd,
    )

examples/deepseek-v4/run_deepseek_v4_flash.sh:41

  • With set -u, if NNODES is set to a value other than 4 or >=8, this script never exports PRIMUS_PP / PRIMUS_EP, but later interpolates them into PRIMUS_EXP_NAME, causing an unbound-variable crash. Add an explicit fallback path that either sets defaults or requires callers to provide the missing env vars.
if [ "$NNODES" -ge 8 ]; then
    export PRIMUS_TP=${PRIMUS_TP:-1}
    export PRIMUS_PP=${PRIMUS_PP:-8}
    export PRIMUS_EP=${PRIMUS_EP:-8}
    export PRIMUS_RECOMPUTE_LAYERS=0

Comment thread .github/workflows/ci.yaml

env:
PRIMUS_TURBO_COMMIT: 56c789e58f72aaf733b7715f1536be1ed33b69a1 # dev/kyle/flydsl_attn_deepseekv4 (dsv4 sparse-MLA attention + cr=4 fixes + flydsl kernels packaging __init__.py fix)
PRIMUS_TURBO_COMMIT: edc8d2ccb0be4888e80ee7c6e765fd3956026a32 # feat: add quantized tensor support for blockwise grouped gemm (#422)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants