fix(recipe): add library aliases for the colliding GLM-5.2 SFT recipes - #5608
Open
pruprakash wants to merge 1 commit into
Open
fix(recipe): add library aliases for the colliding GLM-5.2 SFT recipes#5608pruprakash wants to merge 1 commit into
pruprakash wants to merge 1 commit into
Conversation
glm52_sft_416gpu_h100_bf16_config and glm52_sft_192gpu_gb200_bf16_config are exported by both megatron.bridge.recipes and megatron.bridge.perf_recipes, and bare launcher lookup binds the benchmark definition, so the GLM-5.2 verification card's SFT commands launched a 131,072-token benchmark job instead of the library configuration its published metrics were measured on. Adds a unit test covering the change (red-green verified). Detected by: megatron-bridge QA Signed-off-by: Pruthviraj Prakash <pruprakash@nvidia.com>
Contributor
|
LGTM Clean, well-motivated fix. The recipe names glm52_sft_416gpu_h100_bf16_config and glm52_sft_192gpu_gb200_bf16_config are in BENCHMARK_RECIPE_PRECEDENCE_COLLISIONS, so a bare --recipe launcher lookup resolves them to the benchmark (perf) definitions rather than the library SFT recipes the verification card intends. Introducing the generic aliases glm52_h100_sft_config and glm52_gb200_sft_config, and pointing the card at them, keeps the intended library workloads selectable. Verified:
Suggested test cases:
|
Contributor
Author
|
/ok to test 32c0ad9 |
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
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.
What does this PR do ?
Fixes the GLM-5.2 verification card's two SFT reproduction commands launching a 131,072-token benchmark job instead of the library configuration their published metrics were measured on, by giving those library recipes a distinct name.
Changelog
recipes/glm/h100/glm5.py: addglm52_h100_sft_configalias.recipes/glm/gb200/glm5.py: addglm52_gb200_sft_configalias.recipes/glm/{__init__,h100/__init__,gb200/__init__}.py: export the aliases.examples/model_verification_cards/glm5-2/card.yaml: both SFT commands name the alias.tests/unit_tests/recipes/test_glm5_recipes.py: 4 parametrized cases over the aliases and the shipped card commands.Additional Information
megatron.bridge.recipesandmegatron.bridge.perf_recipes;load_recipebinds the benchmark definition and the library workloads had no other name.nvcr.io/nvidian/nemo:nightly— RED is a collectionAttributeErrorwithout the aliases and2 failedwith the card unchanged; GREEN is20 passed. On the patched treeload_recipereturns gbs=32/seq=2048 (H100) and gbs=8/seq=8192 (GB200), matching the card.BENCHMARK_RECIPE_PRECEDENCE_COLLISIONSentries still lack a library alias, andvalidate_card.pystill does not check--recipe.