Skip to content

feat(flux): core runtime + Megatron adapter scaffolding#807

Merged
wenxie-amd merged 7 commits into
feat/flux/ci-envfrom
feat/flux/core
Jul 7, 2026
Merged

feat(flux): core runtime + Megatron adapter scaffolding#807
wenxie-amd merged 7 commits into
feat/flux/ci-envfrom
feat/flux/core

Conversation

@jasainio

Copy link
Copy Markdown
Contributor

Base of an 18-PR series splitting the Flux diffusion-training feature (training Flux, a DiT text-to-image diffusion model, on Primus/Megatron) out of one large branch for reviewability. Targets feat/flux/ci-env and auto-retargets to main once that merges. Every other content PR stacks on this one.

What this changes

The shared runtime scaffolding the rest of the feature builds on: core runtime state + train-runtime wiring, the Megatron adapter and base/pretrain trainers, and a patch auto-loader (patches/__init__.py) that imports every *_patches.py in the package on import, so each later layer only drops in its own patch file with no registry edit. Also carries the shared test root (tests/conftest.py, tests/utils.py) and a one-line .gitignore change (the bare data ignore → root-anchored /data/*, and nothing else) that stops Git from ignoring the in-repo data/ source and config directories the later layers add. Kept deliberately small so it can land first.

Dependencies

Sequenced after the CI-pins PR (feat/flux/ci-env); no functional dependency on the turbo bump.

Test plan

pytest tests/unit_tests/core tests/unit_tests/backends/megatron; lint/pre-commit clean. Validated locally on an AMD GPU container: 58 passed.

Files

39 (core runtime, Megatron adapter/trainers, base patch loader, shared test root, .gitignore).

… on draft PRs (#806)

Root of an 18-PR series splitting the Flux diffusion-training feature
(training Flux, a DiT text-to-image diffusion model, on Primus/Megatron)
out of one large branch for reviewability. This PR targets `main` and is
the **first to merge**. The content branches are cut from this branch
(`feat/flux/ci-env`), not raw `main`, so they carry the bumped pins +
draft guard on their own heads — which lets the rest of the stack open
as drafts immediately, before this PR merges.

## What this changes
Updates the public-CI dependency pins to the versions the Flux diffusion
feature needs:
- Bumps `PRIMUS_TURBO_COMMIT` to a recent Primus-Turbo `main` build that
exposes the mxfp4 `gemm_fp4_impl(..., preshuffled=...)` fast path, and
`PRIMUS_TURBO_AITER_COMMIT` to AITER `v0.1.14.post1`. Mirrors the same
turbo commit into `benchmark.yaml` (the docker build picks both up
through `ci.yaml`'s build-args).
- Adds a **draft-skip guard** to the expensive jobs (`build-docker`,
`run-unittest-torch`, `run-unittest-jax`) so the rest of the stack can
be opened as Draft PRs without firing the full docker-build + GPU
pipeline on every open/sync; `code-lint` stays ungated. Two parts: add
`ready_for_review` to the `pull_request` trigger `types` (the default
`opened/synchronize/reopened` set omits it, so without this the
draft→ready flip triggers nothing) **and** add `if: ${{
github.event_name != 'pull_request' || !github.event.pull_request.draft
}}` to the three heavy jobs.

## Why it merges first (but need not block opening)
On the current `main` pins the mxfp4 path fails (`gemm_fp4_impl` rejects
the `preshuffled` arg), so content PRs can only go green if their head
branch carries the new pins — which stacking on `feat/flux/ci-env`
provides without a merge. Merging this first matters at *merge* time:
once it's on `main`, children auto-retargeted to `main` inherit the fix
and the guard.

No `MEGATRON_PATH` change is needed: the diffusion unit tests run
in-process and `tests/conftest.py` already puts the `submodules:
recursive`-checked-out `third_party/Megatron-LM` on `sys.path` when
`MEGATRON_PATH` is unset.

## Dependencies
None — this is the root prerequisite.

## Test plan
No unit tests of its own. After the runner rebuilds the image, confirm a
smoke run is green: `pytest tests/unit_tests/backends/megatron/diffusion
-k mxfp4`. The guard can be sanity-checked by opening a throwaway draft
PR (expect only `code-lint`) and marking it ready (expect the full
pipeline).

## Files
2 (`.github/workflows/ci.yaml` — pins + draft guard;
`.github/workflows/benchmark.yaml` — mirrored turbo pin).

---------

Co-authored-by: Flux Split <flux-split@local>
Co-authored-by: WangLingxun <linxwang@amd.com>
Co-authored-by: Xiaoming-AMD <Xiaoming.Peng@amd.com>
Co-authored-by: luiza-amd <Luiza.Sayfullina@amd.com>
@luiza-amd luiza-amd marked this pull request as ready for review July 6, 2026 06:24
Flux Split and others added 2 commits July 6, 2026 01:56
… on draft PRs

Bump PRIMUS_TURBO_COMMIT to Primus-Turbo main (0.3.x, validated build) which
exposes the mxfp4 gemm_fp4_impl(..., preshuffled=...) fast path, and
PRIMUS_TURBO_AITER_COMMIT to AITER v0.1.14.post1. Mirror the turbo commit into
benchmark.yaml; the docker build receives both via ci.yaml build-args.

Add a draft-skip guard to the expensive jobs (build-docker, run-unittest-torch,
run-unittest-jax) so the stacked Flux PRs can sit in Draft without firing the
full pipeline; code-lint stays ungated. Add ready_for_review to the
pull_request trigger types so flipping a draft to Ready triggers the first run.

MEGATRON_PATH export intentionally omitted: the Flux diffusion unit tests run
in-process (no megatron-importing subprocess remains after the te_vs_local
in-process conversion), and tests/conftest.py auto-adds
third_party/Megatron-LM to sys.path when MEGATRON_PATH is unset. CI already
checks out submodules: recursive, so megatron imports without any extra env.

rebase to main branch
@luiza-amd luiza-amd marked this pull request as draft July 6, 2026 07:03
@luiza-amd luiza-amd marked this pull request as ready for review July 6, 2026 09:10
@luiza-amd luiza-amd marked this pull request as draft July 6, 2026 09:48
@luiza-amd luiza-amd marked this pull request as ready for review July 6, 2026 10:11
@zirui zirui self-requested a review July 6, 2026 11:00
@luiza-amd

luiza-amd commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Last 2 commits pushed resolve an issue in some existing pretrainers which feat/flux/core introduced.

On this branch, train_runtime.py was changed to instantiate every trainer with a much richer argument set:

        trainer = trainer_cls(
            backend_args=backend_args,
            module_name=self.ctx.module_name,
            primus_config=self.ctx.primus_config,
            module_rank=self.ctx.rank,
            module_world_size=self.ctx.world_size,
            module_master_addr=self.ctx.master_addr,
            module_master_port=self.ctx.master_port,
        )

Previously this was just trainer = TrainerClass(backend_args=backend_args).

The Megatron trainers survive this because MegatronBaseTrainer has no __init__, so it inherits BaseTrainer.__init__(self, backend_args=None, *args, **kwargs), which cooperatively absorbs/filters the extra kwargs.

  • primus/backends/maxtext/maxtext_pretrain_trainer.py — the one causing the run-unittest-jax CI failure
  • primus/backends/torchtitan/torchtitan_pretrain_trainer.py
  • primus/backends/hummingbirdxt/hummingbirdxt_posttrain_trainer.py
  • primus/backends/megatron/megatron_sft_trainer.py
  • primus/backends/megatron_bridge/megatron_bridge_base_trainer.py
  • primus/backends/megatron_bridge/megatron_bridge_pretrain_trainer.py
  • primus/backends/megatron_bridge/megatron_bridge_posttrain_trainer.py

Each of these trainers had a rigid def __init__(self, backend_args: Any) signature that would raise TypeError under the new train_runtime.py call convention (which now passes module_name, primus_config, module_rank, module_world_size, module_master_addr, module_master_port).

Fix applied

Widen MaxTextPretrainTrainer.__init__ (and the other non-Megatron trainers) to accept and forward the extra kwargs, letting BaseTrainer filter them:

def __init__(self, backend_args: Any = None, **kwargs):
    super().__init__(backend_args=backend_args, **kwargs)

This is safe: for trainers that are not BaseModule subclasses, BaseTrainer.__init__ takes the else: super().__init__() branch and discards the extra kwargs; for BaseModule subclasses it forwards them via super().__init__(**kwargs).

@wenxie-amd wenxie-amd merged commit a272718 into feat/flux/ci-env Jul 7, 2026
7 of 12 checks passed
Xiaoming-AMD added a commit that referenced this pull request Jul 7, 2026
…ing (#856)

**Important**: The content of this PR was approved and merged
#807, but not to main (auto-target
to main was not triggered and the lesson was taken into account for
further PRs).
**The purpose of this PR is to finalize the merge to main and no new
changes were introduced.**

Base of an 18-PR series splitting the Flux diffusion-training feature
(training Flux, a DiT text-to-image diffusion model, on Primus/Megatron)
out of one large branch for reviewability. Targets `feat/flux/ci-env`
and auto-retargets to `main` once that merges. Every other content PR
stacks on this one.

## What this changes
The shared runtime scaffolding the rest of the feature builds on: core
runtime state + train-runtime wiring, the Megatron adapter and
base/pretrain trainers, and a patch auto-loader (`patches/__init__.py`)
that imports every `*_patches.py` in the package on import, so each
later layer only drops in its own patch file with no registry edit. Also
carries the shared test root (`tests/conftest.py`, `tests/utils.py`) and
a one-line `.gitignore` change (the bare `data` ignore → root-anchored
`/data/*`, and nothing else) that stops Git from ignoring the in-repo
`data/` source and config directories the later layers add. Kept
deliberately small so it can land first.

## Dependencies
Sequenced after the CI-pins PR (`feat/flux/ci-env`); no functional
dependency on the turbo bump.

## Test plan
`pytest tests/unit_tests/core tests/unit_tests/backends/megatron`;
lint/pre-commit clean. Validated locally on an AMD GPU container: 58
passed.

## Files
39 (core runtime, Megatron adapter/trainers, base patch loader, shared
test root, `.gitignore`).

---------

Co-authored-by: Flux Split <flux-split@local>
Co-authored-by: WangLingxun <linxwang@amd.com>
Co-authored-by: Xiaoming-AMD <Xiaoming.Peng@amd.com>
Co-authored-by: Flux Split Trial <flux-split-trial@local>
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.

3 participants