Skip to content

Flatten Megatron configs, smoke, and sweep cell keys and update docs - #391

Merged
sukesh-amd merged 5 commits into
mainfrom
training_primus
Sep 9, 2026
Merged

sukesh-amd merged 5 commits into
mainfrom
training_primus

Conversation

@sukesh-amd

Copy link
Copy Markdown
Contributor

Summary

Document unified suites and Primus checkpoint save

  • Move suite and config detail from the long test/config READMEs into published RST (docs/how-to/test-suites/training/megatron.rst and docs/reference/configuration-files/training/megatron.rst).
  • Pass Primus --save on checkpoint writes so resume can load a real checkpoint.

Drive test_smoke from a top-level smoke block

  • test_smoke reads smoke.enabled, iters, micro_batch_size, global_batch_size, and precision from the variant JSON instead of hardcoded knobs.
  • enabled=false skips smoke. All packaged Megatron variant configs include this block.

Flatten Megatron configs onto paths, train_params, and container.env

  • Drop nested config / model_params / schema_version / framework.
  • Use gpu_name, paths, train_params, and container.env (NNODES, MASTER_ADDR, NCCL_*, sockets as docker run -e).
  • Flatten those sections into the job dict at load time. Wrapper scripts no longer re-export NCCL/socket/MASTER_ADDR/NNODES.
  • Threshold specs with "optional": true SKIPPED in test_metric when the metric is missing (training.mem_usage; distributed also training.scaling_efficiency_pct).
  • Checkpoint I/O parser matches Primus loading distributed checkpoint from as well as Megatron-LM loading checkpoint from.
  • Keep how-to and schema RST in sync with the flattened layout.

Require sweep keys to match MBS/GBS/PRECISION

  • Rename every packaged sweep.combinations key and matching sweep.runs entry to MBS=<micro_batch_size>,GBS=<global_batch_size>,PRECISION=<precision> (same string as the threshold cell and the pytest parametrize ID).
  • Config load fails if a combination key does not match those three fields, so pytest IDs and threshold lookup cannot drift.
  • Log directories still sanitize = and , to _ (for example MBS_4_GBS_128_PRECISION_FP8).
  • Update how-to and schema RST examples to the new keys.

Test plan

  • make fmt-check lint ut
  • Load a filled *_single.json and *_distributed.json via load_training_variant (all <changeme> replaced)
  • Single-node: smoke from smoke.*; one sweep cell; optional mem_usage SKIPPED if the parser does not emit it
  • Distributed Primus: checkpoint save/resume with --save; load log contains loading distributed checkpoint from
  • Confirm combo keys are MBS=…,GBS=…,PRECISION=…; a leftover slug like llama3_1_8b-mi300x-… fails load

Move suite and config detail into published how-to and schema RST, and pass --save on Primus checkpoint writes so resume can load a real checkpoint.
Skip optional missing metrics, parse Primus distributed checkpoint load lines, and keep the docs in sync.
Align combination ids with threshold cells so pytest IDs and lookup keys cannot drift.
@sukesh-amd
sukesh-amd requested a review from solaiys September 8, 2026 20:02

@solaiys solaiys left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: Primus save is a checkpoint directory, not a boolean flag. Bare --save will be parsed as the path --save_interval and the actual interval token is left dangling, so checkpoint write/resume still cannot work.

Comment thread cvs/lib/training/megatron/primus_lib.py
@sukesh-amd
sukesh-amd requested a review from solaiys September 9, 2026 06:58
"NCCL_IB_HCA": "<changeme>",
"NCCL_SOCKET_IFNAME": "<changeme>",
"GLOO_SOCKET_IFNAME": "<changeme>",
"NCCL_IB_GID_INDEX": "3",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this also should be <changeme> with example.
Either give the example in a comment line key:value.
OR
add the <changeme> tag at the end of the actual value what we are using

ex:
"NCCL_IB_HCA": "rdma0,rdma1,rdma2,rdma3,rdma4,rdma5,rdma6,rdma7 <changeme>",
"NCCL_SOCKET_IFNAME": "eno0 <changeme>",
"GLOO_SOCKET_IFNAME": "eno0 <changeme>",
"NCCL_IB_GID_INDEX": "3 <changeme>",

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

@solaiys solaiys left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changeme tag should accompany with the example values.
submit it in the next PR.

@sukesh-amd
sukesh-amd merged commit 5eb5aee into main Sep 9, 2026
2 checks passed
@cijohnson
cijohnson deleted the training_primus branch September 15, 2026 00:10
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.

2 participants