fix: verify complete Megatron patches before updating installations - #208
Merged
Merged
Conversation
hjh0119
approved these changes
Sep 23, 2026
FunJim
added a commit
to FunJim/mcore-bridge
that referenced
this pull request
Sep 23, 2026
Include upstream main through cef925c (PR modelscope#208). Retain the fork's full-file hash installer and matching tests because this branch pins the exact Megatron baseline; upstream intentionally supports source drift and three-way merges. Runtime and tests are identical to GPU-validated b567996. Record the completed single-GPU and initialized CP8 regressions. Signed-off-by: FunJim <[email protected]>
FunJim
added a commit
to FunJim/mcore-bridge
that referenced
this pull request
Sep 23, 2026
Use the installer and safety tests merged in ModelScope PR modelscope#208. Keep only the test assertion for the fork's runtime-only patch bundle. Retain all GLM runtime fixes without changes and document hunk verification, source three-way merging, and single-process environment preparation. Validation: 20 CPU tests passed without skips, including the baseline Megatron wheel and source. Built and non-editably installed the bridge wheel; apply/check/repeat and package discovery passed. All 14 patched file blob hashes match for wheel and source. Flake8, isort, YAPF and diff checks passed. Signed-off-by: FunJim <[email protected]>
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.
Problem
The GLM patch installer treats the presence of
kda_two_stage_gatesin one file as proof that the whole patch is installed. A partially applied or outdated patch can therefore be reported as complete. The bundled diff also contains a Megatron unit-test file that is absent from installed wheels, and a failed application can leave earlier files modified.Change
--checkand--root.megatron.core.This remains an environment-preparation tool: run it once in an unused environment, not from training ranks or while another process is editing the installation.
Validation
20 CPU tests passed, using real
gitandpatchexecutables. Coverage includes partial files/hunks, conflicts, failed application/writeback, symlinks, repeated application, unrelated source edits, three-way merges, index preservation, and wheels nested inside another Git repository.Two optional integration tests exercised the complete bundled patch against Megatron
ee743d3ef228f546287dc3835c2cf56011d5136b: its built wheel and baseline source files in a temporary Git repository. All 11 runtime files / 12 source files matched the patch's expected post-application blob hashes. SetMEGATRON_PATCH_BASE_WHEELandMEGATRON_PATCH_BASE_SOURCEto enable these tests:Also built and non-editably installed the bridge wheel with
--no-deps, then verified apply/check/repeat against the installed Megatron wheel without importing its native modules. Flake8, isort, YAPF andgit diff --checkpassed for the changed files. No model execution or GPU training was needed; runtime model/optimizer patches are unchanged.