build: bump Megatron-LM to the MoE expert-LoRA + hybrid-recompute stack - #4
Closed
taufeeque9 wants to merge 29 commits into
Closed
build: bump Megatron-LM to the MoE expert-LoRA + hybrid-recompute stack#4taufeeque9 wants to merge 29 commits into
taufeeque9 wants to merge 29 commits into
Conversation
The NVIDIA CI substrate does not exist in this repo: no copy-pr-bot (so pull-request/N branches are never created), no self-hosted H100/GB200 runners, no org secrets or vars, and no access to the NVIDIA container registries. 24 of the 26 workflows can therefore never run here. Several were also actively harmful rather than merely inert: cicd-approve-test-queue ran on a */5 cron, creating ~288 empty runs a day, and dependabot invoked _update_dependencies, which pushes directly to the default branch and deletes remote branches via secrets.PAT. Keep the only two workflows that run on GitHub-hosted runners without org secrets: detect-secrets and link-check. Retain workflows/config/.secrets.baseline, which detect-secrets depends on for its audited-findings allowlist. Scope link-check to the Sphinx sources under docs/: drop the Fern --root-dir and mdx glob, and drop the kimi-k3 excludes now that the PR that added those files (NVIDIA-NeMo#5130) is merged and blob/main resolves. Also remove CODEOWNERS, which routed every path to NVIDIA teams that do not exist in this org and so silently no-opped, and copy-pr-bot.yaml, which is inert without the bot. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Tigist Diriba <tigist@far.ai>
Builds out the fork's own CI on FAR.AI infrastructure, replacing the NVIDIA-only workflows removed in 7768fb52. GPU CI (gpu-tests*.yml, .github/k8s/, .testignore) A PR comment "@flamingo run gpu-tests [core|diffusion]" dispatches an ephemeral 2-GPU Kubernetes job on the H100 cluster that runs the two unit-test launchers and owns a gpu-tests/<slug> commit status. A gate workflow auto-passes the required status for PRs that only touch .testignore-matched paths, read from the base branch so a PR cannot exempt its own files. Only the unit suites are wired up; the functional tests need NVIDIA's /home/TestData volume and the gb200 tree targets hardware we do not have. Local runners (tools/, k8s/, Makefile) test_on_flamingo.sh runs the same launchers, in the same image, on an ephemeral GPU pod against your working tree, so uncommitted changes can be tested without a PR. Runs are detached on the pod and survive disconnects. build_image_on_flamingo.sh builds Dockerfile.ci on the cluster's shared BuildKit and pushes to GHCR, since the build compiles DeepEP and runs a full uv sync. Both are driven from the Makefile. Lint CI (pre-commit.yml) Restores the pre-commit run that lived in cicd-main.yml, on ubuntu-latest. Invoked directly rather than through "uv run --group dev", whose nvidia-resiliency-ext dependency is Linux-only and cannot install on macOS. rsync in Dockerfile.ci The only patch to an upstream-maintained file. The sync transport runs "rsync --server" inside the container, so rsync must exist there; without it the runner falls back to re-sending the whole tree (~258 MB with .git) every run. Appended after the expensive layers so it does not invalidate the DeepEP / uv sync cache. Verified end to end on the cluster: image built and pushed, then both suites green on a 2-GPU pod (9099 passed core, 533 passed diffusion, rc=0). Three defects found and fixed while getting there, all in the sync path rather than in the tests: - .git was never synced, so the examples/**/slurm_conversion.sh wrappers ("git rev-parse --show-toplevel") and test_mcore_commit ("git ls-tree") failed with exit 128. CI clones and so has a real repo; the local path did not. - macOS tar emits an AppleDouble "._<name>" companion per file, which GNU tar materializes in the pod; code that globs sources and calls read_text() then died with UnicodeDecodeError, failing 21 tests. Fixed with COPYFILE_DISABLE=1 in both syncs. Note this cannot be verified with "tar -tzf" on macOS, which re-merges those entries. - rsync was absent from the image, silently forcing the tar path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Tigist Diriba <tigist@far.ai>
build_adapter_conversion_tasks picks ColumnParallelMapping for linear_out on every parallel base layer. Mamba's in_proj base export is gathered component-major by MambaInProjMapping (z/x/B/C/dt each gathered across TP ranks, then concatenated), so a rank-major adapter gather lands the LoRA delta on the wrong rows whenever tensor-parallel size > 1. The shapes still agree, so this is silent. Measured by executing the real distributed merge on basis vectors and tracking row provenance: 0/104 rows misplaced at TP=1, 84/104 at TP=2 (max abs err 37.6), 99/104 at TP=8 (err 482.5). With this change: 0/104 at both TP=2 and TP=8. Gated on isinstance(base_mapping, MambaInProjMapping) rather than a name match: GDN in_proj (self_attention.in_proj) has its own de-interleaving merge branch that deliberately consumes a rank-major linear_out, so a substring match on "in_proj" would double-correct it and corrupt Qwen3-Next exports. megatron_to_hf_lookup returns Optional, so the gate is None-safe and falls back to existing behaviour. Observed downstream in RL training on Nemotron-3-Super-120B (TP=2): the vLLM policy receives a scrambled in_proj delta, so it tracks only ~9-30% of the trainer's movement while a TP=1 control tracks 1.0x.
Fused-expert mappings (is_grouped_export) accumulate one expert per task and stack them into a single HF tensor. The export loop returned early on that path, before the adapter merge, so a LoRA targeting an expert weight was silently dropped — the stacked base tensor is still the shape HF expects, so nothing downstream could notice. Merge before accumulation: after stacking, the per-expert adapter can no longer be matched to its slice. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Both comments explained what the previous code did wrong. A reader here needs what the code must do: linear_out gathers like the base weight it merges into, and the grouped merge happens before accumulation stacks the experts. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The grouped-export LoRA test came in via cherry-pick from contrib and was never run through ruff-format, so it fails the pre-commit CI added in this branch. Reflow only — the AST is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Tigist Diriba <tigist@far.ai>
Three related changes to the image tooling, plus a rename to make the
target names consistent.
Tagging. A build now publishes an immutable sha tag and a moving branch
tag (:3995ef57a, :farai-main) instead of writing :latest. The sha tag
carries a -dirty suffix when the working tree is not clean, because the
build context IS the working tree — Dockerfile.ci COPYs it rather than
cloning a ref, so a bare sha would claim a provenance the image does not
have.
Promotion. :ci and :latest are what CI pods and other people pull, so a
routine build must not repoint them. They move only via PROMOTE=ci /
PROMOTE="ci latest" during a build, or the image-promote-* targets
afterwards. Inline promotion adds --tag flags to the same push so every
tag lands atomically; the standalone targets use "docker buildx
imagetools create", a registry-side manifest copy that needs no pull.
PROMOTE accepts only ci and latest so a typo fails loudly, and it is
refused from a dirty tree (override: ALLOW_DIRTY_PROMOTE=1) since a
-dirty image is reproducible from no commit. Dirty builds stay allowed.
The CI pod now pulls :ci rather than :latest. Previously any developer
running a build silently changed what CI ran.
Detached builds. The remote build wrote its output through a single long
kubectl exec, so any network blip killed a ~90 minute build — one died
at stage 10/12 this way. It now writes a runner to the pod, launches it
under setsid with a log file and exit-code sentinel, and polls with short
independent execs, mirroring what tools/test_on_flamingo.sh already does
for tests. Re-attach with image-build-logs, manage with image-build-list
and image-build-teardown; FOLLOW=0 launches and returns, KEEP=1 leaves
the job up. The builder job name gained a random suffix so concurrent
builds cannot collide, the pod gained discovery labels, and it gained an
activeDeadlineSeconds backstop because an abandoned detached build had
nothing to reclaim it.
The runner is written with `tee`, not `sh -c "cat > file"`. The builder
pod is alpine (docker:27-cli) and its busybox ash consumes the piped
stdin while parsing, so cat sees EOF and writes a ZERO-BYTE file; `sh`
then runs an empty script, exits 0 in milliseconds, and the whole build
reports success while building nothing. tools/test_on_flamingo.sh gets
away with the cat form only because its pod is Ubuntu-based. Three
guards now make that class of silent no-op impossible: the runner must
be non-empty, it must parse as shell, and every tag must actually
resolve in the registry before the script claims success — an exit code
of 0 is no longer treated as proof that anything was pushed.
Naming. image-remote/image-local become image-build-remote/-local, the
new management targets are image-build-{list,logs,teardown}, and
promote-* become image-promote-*. This matches the test family's
<domain>-<what>-<variant> shape (test-unit-remote) and keeps one prefix
per domain. buildx-builder-local is left alone: it is an internal
prerequisite of image-build-local that nobody invokes directly.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Tigist Diriba <tigist@far.ai>
This fork is consumed as a submodule by NeMo-RL, which needs to know whether a
given commit here is compatible with a given commit there. Compatibility is a
pure function of git data:
compatible(N, D) <=> base(D) == pin_P(base(N))
base(X) = merge-base(X, upstream/<branch>)
so no tagging scheme or external database is needed. But consumers declare their
submodules `shallow = true`, and a shallow clone has no history to compute a
merge-base from, so the base has to be recorded rather than derived.
`.fork-base.json` records it, and it records only fields that are stable across
the patch series. The fork head and patch SHAs are deliberately omitted: they
would force a regeneration on every commit and could never be accurate inside
the very commit that carries them. In practice `upstream_base` changes only on a
rebase.
Recorded metadata is worth nothing unless something enforces it, and the moment
it goes stale is a rebase — exactly when nobody is thinking about it. The
fork-base workflow therefore recomputes the merge-base on every PR and on pushes
to farai/main, publishing the status context `fork-base` so it can be marked
required. It carries no `paths:` filter on purpose: a required context that is
skipped for some PRs never reports, and those PRs stay blocked forever.
tools/fork_base.py is written to be dropped unchanged into any fork that carries
patches on an upstream. A repo may be a dependency, a consumer, or both, and the
roles are modes of the same computation — this repo is both, since it is
consumed as a submodule and itself pins 3rdparty/Megatron-LM. `--check` enforces
four things:
1. the manifest still equals merge-base(HEAD, upstream/<branch>)
2. every pinned submodule is compatible (patched: recorded base matches
upstream's pin; unpatched: gitlink matches exactly)
3. a dependency's manifest names the fork we pin and the upstream we expect —
two forks can share a base while carrying different patch sets
4. a dependency's nested submodules pin what we pin, matched by URL rather
than path, since layouts differ between repos
Rule 4 is not redundant: a patch can move a nested gitlink without moving the
dependency's base, so rule 2 alone reports green while the pins have diverged.
Megatron-LM is exactly this shape, pinned both here and by our consumer.
Dependencies that cannot be verified are failures rather than skips, so a CI job
that forgot `submodules: recursive` cannot pass while checking nothing;
`--allow-skips` opts out for local use.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Tigist Diriba <tigist@far.ai>
…flow
The compatibility rules assume our tree is "upstream@base plus our patches",
but they cannot see whether that is true. A merge FROM upstream satisfies every
one of them while breaking the assumption: merge-base reports the newest
upstream commit we contain, so the manifest records it, yet conflict resolution
may have taken --ours and quietly dropped upstream hunks. Replaying the patches
makes the claim literally true, because they are applied to upstream's exact
tree.
--check now also enforces the shape of history:
5. the patch series is linear. A merge from upstream always fails. Internal
merges fail by default too — harmless to the compatibility rules, but a
plain rebase drops each of their resolutions, which is what makes replaying
a series onto a new base expensive. --allow-merges downgrades that second
half for a fork that is not linear yet.
6. exactly one merge-base with upstream, so merge-base cannot pick arbitrarily
between several and leave the recorded base non-deterministic
7. the base is a genuine upstream commit, not one invented locally
8. with --against <ref>, the base only moves forward. This needs both sides,
so CI applies it only on PRs; it is what stops an accidental rebase
backwards onto older upstream.
Upstream merges are detected as "a parent that upstream contains", deliberately
not as "a parent in upstream but not in our base". Merging upstream moves the
base to the very commit that was merged, so under the latter phrasing that
parent becomes an ancestor of the new base and the merge slips through — the
exact case the check exists to catch. The fixture caught this; --allow-merges
was waving upstream merges past. A purely internal merge cannot trip the new
form, since both its parents carry our patches and so are not contained in
upstream.
make sync-upstream replays the series onto a newer upstream commit on a
sync/upstream-<YYYYMMDD>-<short-sha> branch and regenerates the manifest. The
branch name is a checkable claim: CI asserts the SHA in it equals the recorded
upstream_base. It stops before pushing on purpose — none of GitHub's merge
buttons produce a linear replay (a merge commit is non-linear, squash collapses
the series, and "rebase and merge" replays onto the target tip and duplicates
the patches), so the branch is reviewed as a PR and landed by force-pushing the
reviewed SHA. The push trigger on farai/main then re-runs these checks, so a
force-push that was not the reviewed commit turns the branch red.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Tigist Diriba <tigist@far.ai>
These survived the earlier prune only because rebasing onto an earlier upstream
base changed which files existed, so that commit's deletions did not cover them.
They are the same class it removed: infrastructure this fork does not have.
build-test-publish-wheel.yml triggers on pushes to main and wants
NVIDIA_MANAGEMENT_ORG_PAT, SVC_PYPI_TEST_TOKEN
and TWINE_PASSWORD
build-docs.yml same triggers, builds docs through NVIDIA's
FW-CI-templates, which we do not publish through
remove-needs-author.yml pull_request_target + issue_comment, managing a
needs-author label from upstream's review process
The first two were reachable rather than dormant: `main` here is the clean
upstream mirror branch, so pushes to it would have fired them. A publish path
wired to secrets that do not exist is worse than no publish path.
Neither README matched the tree any more: seven workflows now, not six, and the
base carries 20 upstream workflows rather than the 26 of the later base the text
was written against, so 18 were removed rather than 24. Also corrected the
detect-secrets description — it is a thin caller of NVIDIA's reusable
_secrets-detector.yml, and the baseline allowlists audited findings for it
rather than being read by the workflow directly.
Verified before deleting: none of the three is referenced anywhere else in this
repo, and their only file references are remote reusable workflows in
NVIDIA-NeMo/FW-CI-templates. config/.secrets.baseline is untouched.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Tigist Diriba <tigist@far.ai>
It crawled external URLs in `docs/**/*.md` on every PR. That is upstream's own content, which upstream already link-checks on its own repo, so we were paying to re-verify links we do not write. Meanwhile the lychee args were scoped to `docs/`, so the markdown this fork actually maintains — README.farai.md, .github/workflows/README.md, and the root READMEs — was never covered at all. The check we were running was the one we needed least. It also ran with `fail: true` and no `paths:` filter, so every unrelated PR depended on third-party sites being reachable. `--accept '...,401,403,429'` and three retries soften that, but a 5xx or a timeout still reds a PR that changed no documentation. The weekly cron went with it. Nothing here publishes docs yet (the Pages workflow is still deferred), so external link rot in upstream's docs is not currently our problem to detect. Also corrects the detect-secrets description in both READMEs: it is a thin caller of NVIDIA's reusable _secrets-detector.yml, and the baseline allowlists audited findings for that workflow rather than being read by this one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Tigist Diriba <tigist@far.ai>
A PR touching only the Makefile, tools/ or the top-level k8s/ pod specs cannot change what the unit suites do, but it still burned an H100 run to merge. gpu-tests.yml drives the suites through kubectl against .github/k8s/pytest-multigpu-job.yaml and references no make target, no tools/ script, and nothing under the top-level k8s/ — that directory holds the component: local-dev pod specs used by `make test-unit-remote` and `make image-build-remote`, which CI never touches. The image CI runs is pulled by the :ci tag, which moves only on an explicit promotion, so even the image-build tooling cannot change a CI result by being edited. Listed file by file, following the convention the workflow entries already use. A bare `tools/` would silently exempt any future tool that DOES affect the suites, and gitignore cannot re-include a path underneath an excluded directory, so the mistake would not be correctable by a negation. Verified in both directions: src/, tests/unit_tests/, gpu-tests.yml, .github/k8s/pytest-multigpu-job.yaml and docker/Dockerfile.ci all remain gated. Note this does not affect the PR that introduces it — the gate reads .testignore from the base branch, so these entries take effect only once merged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Tigist Diriba <tigist@far.ai>
FAR.AI runs secret scanning at the GitHub Enterprise level, so this duplicated coverage we already have. It was also awkward to live with. The workflow was a thin caller of NVIDIA's FW-CI-templates/_secrets-detector.yml, so its scan arguments lived upstream and the only tuning surface here was config/.secrets.baseline. That became a liability the moment .fork-base.json existed: a 40-character git SHA is by construction a "Hex High Entropy String", so it flagged the manifest and the example in README.farai.md, and because the recorded SHA changes on every rebase, a per-secret baseline entry would have gone stale immediately — making a re-baseline part of every upstream sync. The baseline goes with it. Nothing else read it: no pre-commit hook, no other workflow. With this gone no upstream workflow remains, so the framing in both READMEs changes rather than just the counts — all five remaining workflows are FAR.AI additions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Tigist Diriba <tigist@far.ai>
The forward-only check reads .fork-base.json from the PR's target branch to confirm the base moved forward. On the PR that introduces the manifest there is nothing to read, and the code returned that as a problem — while its own message said it was skipping the check. So the very first PR failed the check it was adding. Having nothing to compare against is the absence of a comparison, not a violation. check_forward now returns (problems, notes) and reports this as a note. A manifest that exists but cannot be parsed is still a failure: that is a real fault rather than a missing baseline, and the old code conflated the two under "could not read". This is not a one-off. The same shape recurs in nemo-gym and nemo-rl the moment the tool is rolled out there, so the fixture gained a case for it (21 tests). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Tigist Diriba <tigist@far.ai>
Force-pushing farai/main is banned, so a rebase of the shared branch cannot
land. Merging advances the base without rewriting anything: upstream's commits
keep their SHAs and become reachable from our HEAD, so merge-base moves forward
to the newest upstream commit we contain — which is exactly what the manifest
records. Nothing anyone has cloned is invalidated, and the sync lands through a
normal PR rather than a force-push nobody can gate.
sync_upstream.sh now merges rather than rebasing. Sync PRs must land with
"Create a merge commit": a squash produces a single commit whose only parent is
farai/main, so upstream's commits never enter our ancestry — the files would
update while merge-base stayed put, leaving the manifest disagreeing with git.
Every PR here now lands as a merge commit, so that is the rule rather than an
exception for syncs.
Two checks went with the rebase model:
- The internal/upstream merge distinction. It existed because merges make a
rebase expensive, each dropped resolution having to be redone. Nothing
replays the series any more, so the cost is gone; --allow-merges went with
it, which also removes the flag nemo-rl would have needed for its 84 merges.
- "the base is a genuine upstream commit". This was unreachable: base_n comes
from `git merge-base`, which returns an ancestor of the upstream ref by
definition, so the check could never fail.
What remains is what can actually go wrong: exactly one merge-base, and the base
only moving forward.
README.farai.md also gets an accuracy pass. Beyond the rebase-to-merge wording,
a full read found two unrelated stale claims: docs/fern/ was described as a
pipeline we do not use, but it was deleted in the original prune; and CI was
said to pull :latest when it pulls :ci, which contradicted the tagging table
three paragraphs below. Adds a mermaid gitgraph showing the base advancing on a
sync, verified by rendering it rather than trusting the syntax, and drops the
Landed column from the patches table.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Tigist Diriba <tigist@far.ai>
feat(fork-base): record the upstream base and verify fork compatibility
Conflict resolutions: - models/conversion/model_bridge.py: both sides independently fixed the same bug, LoRA adapters silently dropped on the is_grouped_export path. Took upstream's fix (3cf7a4f, NVIDIA-NeMo#3341), which supersedes ours (b922ae6): it derives the expert index from task.global_param_name rather than the group-keyed HF name, so it also slices fused grouped adapters and handles EP>1, and it memoizes materialized adapter weights per base prefix. Kept our call-site comment explaining why the merge must precede accumulation. - tests/unit_tests/models/test_model_bridge_lora.py: added weight_dtype, export_hook, and mapping.ep_size to the grouped-export task stub, which the merged export loop now reads. These come from unrelated upstream changes in this sync, not from the conflict. - docker/Dockerfile.ci: kept both sides. Our rsync layer stays above upstream's diffusion-deps block so edits to install_diffusion_deps.sh do not invalidate it. Took upstream's COPY --chmod=644 (209f2bb, NVIDIA-NeMo#3431). - .github/: kept this fork's deletion of 19 NVIDIA-only CI files that upstream had modified. They were removed deliberately in 032c79e and f596b4c because the NVIDIA CI substrate (copy-pr-bot, self-hosted H100/GB200 runners, org secrets, NVIDIA registries) does not exist here. Not verified: the grouped-export unit test could not run on this machine (nvidia-resiliency-ext has no macOS wheel) and the CI image was not built. Signed-off-by: Tigist Diriba <tigist@far.ai> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Local analysis scratch space; never committed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Tigist Diriba <tigist@far.ai>
make test-shell-remote provisions (or reuses — the job name is stable per user) a 2-GPU pod, rsyncs the working tree, and attaches for iterative test runs; re-running re-syncs into the live pod. Also exclude the .claude/.agents symlink farms from both sync transports: the image carries real directories at those paths, which neither rsync nor tar can replace with a symlink, and the tests never read them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Tigist Diriba <tigist@far.ai>
Expert LoRA computes wrong results whenever Megatron suppresses a linear's own tensor-parallel collectives: at expert_tensor_parallel_size > 1 the default adapter's fc2 delta is built from unsummed partials and over-counted by the dispatcher's expert-TP reduce, its fc1 dL/dA silently loses every cross-rank term under an exact forward, the grouped per-expert adapter's bare all_gather severs autograd entirely, and shared-expert fc2 deltas under moe_shared_expert_overlap (plain TP > 1) are counted once per rank — same defect in MultiLoRALinear. Three test layers, each defect demonstrated as xfail(strict=True) so this commit lands green while recording the failures: torch-only closed-form algebra with negative controls; a CPU suite driving the real adapters through real MCore explicit_expert_comm suppression (one-rank gloo init plus mock group sizes; only the collectives are faked, and each fake asserts it received exactly the tensor the real collective would see); and a two-rank TP=2 x ETP=2 real-NCCL suite with its L0 launch script. Verified on 2x H100: every demonstration fails on its intended assertion against this tree (5 passed / 8 xfailed CPU; 4 passed / 7 xfailed distributed). The algebra files and the defect analysis originate from Mohammad Taufeeque's earlier investigation; the files are ported verbatim apart from docstrings. Co-authored-by: Taufeeque <9taufeeque9@gmail.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Tigist Diriba <tigist@far.ai>
…ommunication Complete the adapter arithmetic that explicit_expert_comm suppression leaves undone, using autograd-visible collectives on the groups each adapter already carries: reduce_from(copy_to(x)) over expert-TP for the row-parallel-input case (all-reduce on both passes), copy_to(x) for the column-parallel case (backward-only all-reduce restoring the suppressed dL/dz sum), and a differentiable last-dim all-gather (backward reduce-scatter-sum) replacing the grouped adapter's autograd-invisible bare all_gather in all three backends. On the output side, keep linear_out unsharded-gather-free and zero-embed the local shard at its rank offset, so the downstream sum (the dispatcher over expert-TP; SharedExpertMLP.post_forward_comm over dense TP) counts the delta exactly once with no scaling factor; MultiLoRALinear gets the same treatment on suppressed-comm row-parallel bases. Where no correct completion exists without new machinery, refuse loudly instead of training wrong: DoRA on expert or suppressed-comm TP-sharded linears, SharedOuterGroupedExpertAdapter at ETP>1, expert-adapter dropout at ETP>1 (per-rank RNG on dispatcher-replicated tokens), and LoRALinear.weight on grouped adapter classes. BREAKING CHANGE: the four configurations above now raise NotImplementedError where they previously ran with silently wrong numerics (or an accidental AttributeError). Checkpoint format, weight layout, and export paths are unchanged. The strict-xfail markers from the previous commit are deleted here; every demonstration now asserts for real. Verified on 2x H100 (--runxfail, TF32 off): CPU suite 13/13, distributed 11/11 on both ranks, refusal and wiring tests 26 passed, full peft regression 419 passed. At ETP=1 every new branch short-circuits at group size 1; peak memory is unchanged by construction (the embed buffer replaces the removed gather buffer). The fix design originates from Mohammad Taufeeque's earlier implementation of the same defect class. Co-authored-by: Taufeeque <9taufeeque9@gmail.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Tigist Diriba <tigist@far.ai>
The sync merged in 15 workflow files upstream created after the fork point — new files merge cleanly, so the NVIDIA-only-CI purge (032c79e) never saw them. None belong here: the nvskills pair solicits NVIDIA CI (one via pull_request_target), the Fern suite publishes docs this fork does not build, the NeMo-RL/verl weeklies would fail on schedule every Monday against runners and secrets the fork lacks, and link-check returns in its Fern/mdx form with a cron after farai/main deliberately dropped it (38afde8). Their support script dirs are referenced by nothing else. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Tigist Diriba <tigist@far.ai>
The gate exported the changed-file list into the environment of its Python filter; a large PR (this upstream sync changes ~2,400 paths) overflows the exec argv+envp limit and python3 dies with 'Argument list too long' (exit 126) before any decision is made. Write both lists to RUNNER_TEMP files and read them from Python instead. Semantics unchanged: empty or blank-only .testignore still means nothing is ignored. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Tigist Diriba <tigist@far.ai>
…c565c9a0 sync: merge upstream 0c565c9; make expert LoRA exact at ETP>1
Points the nested megatron-core at tf-at/moe-lora-stack-on-d12f6c8c (AlignmentResearch/Megatron-LM @ e8c17b92b5c): the frozen torch grouped expert GEMM backend, expert_fc1_act selective recompute, fused shared-expert LoRA, and the frozen-embedding requires_grad guard in checkpointed_forward, ported onto the previously pinned upstream base d12f6c8c. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Taufeeque <taufeeque@far.ai>
Signed-off-by: Taufeeque <taufeeque@far.ai>
Signed-off-by: Taufeeque <taufeeque@far.ai>
Collaborator
Author
|
Codex-authored closure note: Closing this as obsolete. |
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
2 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.
Bumps the nested
3rdparty/Megatron-LMpin from upstreamd12f6c8ctotf-at/moe-lora-stack-on-d12f6c8c(reviewable as AlignmentResearch/Megatron-LM#1, against the fork's newfarai/mainintegration branch) (e8c17b92b) — the fork's MoE/expert-LoRA feature stack ported onto that exact base (9 cherry-picks with provenance lines):moe_expert_gemm_backend) + weight-view validation + zero-token/dtype/BF16 test contractsexpert_fc1_actselective recompute (guarded: rejected underuse_transformer_engine_op_fuser, where it would be silently ignored)requires_gradguard inmegatron/core/recompute.py::checkpointed_forward— without it, full-granularity recompute with adapter-only training (frozen embeddings) detaches every chunk and adapters get zero gradient; needed for any LoRA run using full recompute on the hybrid stackmoe_expert_gemm_backend='torch'×moe_single_grouped_weight=Truecombination instead of anAttributeErrorWhy: NeMo-RL main consumes megatron-core through this pin; the Nemotron-3-Super hackable-env RL runs (expert-LoRA, and ≥32k context via full recompute — see AlignmentResearch/nemo-rl#60) need these features, previously only on the old standalone Megatron-LM submodule that main removed.
Do not merge before GPU validation — the port was verified statically only. Named validation items: the FC1-recompute parity/call-count test, the torch grouped-GEMM path end-to-end (needs SM100+), the deeper-nested activation-offload branch, and FP8+frozen-embedding interaction with the new guard. Plan: run the nemo-rl topology benchmark (selective + full recompute arms, 16k/32k) on the B200 reservation against this pin.
🤖 Generated with Claude Code