docs: rewrite README as concise landing page and add architecture diagrams by coketaste · Pull Request #161 · ROCm/madengine

coketaste · 2026-07-16T16:12:22Z

Summary

Rewrite README.md as a concise landing page, replacing the long-form content with a tighter overview (734 → much shorter, ~460 net removed)
Add Mermaid architecture diagrams to docs/README.md (CLI → orchestration → execution/deployment → launcher flow) and docs/deployment.md (build/deploy workflow and target-inference
decision tree), replacing ASCII-art diagrams
Add capability reference tables to docs/launchers.md: per-launcher parallelism support (TP/PP/DP/CP/FSDP/EP) and per-target infrastructure features (local/K8s/SLURM)
Remove now-unused docs/img/architecture_overview.png and docs/img/distributed_workflow.png (superseded by inline Mermaid diagrams)

Test plan

Render README.md, docs/README.md, docs/deployment.md, and docs/launchers.md on GitHub (or a Markdown/Mermaid previewer) to confirm diagrams and tables render correctly
Verify no remaining links reference the deleted PNG files

…ures Trim root README from 707 to 258 lines by moving deep reference material (profiling tables, extended config/usage recipes, tips) into docs/ and linking out. Replace the ASCII architecture block and the stale, unreferenced docs/img PNGs with accurate inline Mermaid figures for the layered architecture, build->run->report pipeline, and deployment-target inference. Move the parallelism/infrastructure capability matrices into docs/launchers.md and add Mermaid workflow diagrams to docs/deployment.md and docs/README.md. Co-Authored-By: Claude <noreply@anthropic.com>

Copilot

Pull request overview

This PR refreshes madengine’s documentation set to better reflect the current CLI/config surface area and to provide clearer architecture/workflow guidance (including Mermaid diagrams and updated examples across local/K8s/SLURM usage).

Changes:

Rewrites and streamlines the top-level README.md, adding Mermaid architecture/workflow diagrams and tightening links to the docs.
Updates multiple docs to match current CLI flags, config keys, and JSON manifest/config shapes (database/reporting/profiling/deployment/launchers).
Renames/standardizes a set of example config fields (notably distributed launcher names and port fields) across K8s/SLURM examples.

Reviewed changes

Copilot reviewed 33 out of 35 changed files in this pull request and generated 13 comments.

Show a summary per file

File	Description
`README.md`	Replaces long-form README content with a tighter landing page + Mermaid diagrams and refreshed examples/links.
`src/madengine/reporting/README.md`	Updates reporting module usage/examples and clarifies which helpers are used by the modern CLI.
`src/madengine/execution/README.md`	Updates execution-layer examples/notes to reflect current APIs and helper modules.
`src/madengine/database/README.md`	Re-documents MongoDB ingestion as an active module, including CLI mapping and API usage.
`docs/README.md`	Adds inline Mermaid architecture diagram matching the main README.
`docs/deployment.md`	Replaces ASCII workflow with Mermaid and clarifies target inference + precedence.
`docs/usage.md`	Refreshes end-to-end usage examples and aligns CLI flags and manifest example shape.
`docs/cli-reference.md`	Updates CLI reference defaults/flags and expands `database` command documentation.
`docs/configuration.md`	Updates configuration guidance (ROCm path semantics, timeouts, credentials examples, etc.).
`docs/launchers.md`	Updates launcher docs, adds capability matrices, and refreshes configuration examples.
`docs/profiling.md`	Updates profiling preset references and example env var naming/documentation.
`docs/installation.md`	Minor cleanup (duplicate command line removal).
`docs/contributing.md`	Fixes test path in contributor instructions.
`docs/batch-build.md`	Updates manifest key documentation and adds clarifying note about `deployment_config`.
`examples/slurm-configs/README.md`	Updates example references/paths and documents node health preflight flags.
`examples/slurm-configs/minimal/megatron-lm-minimal.json`	Updates launcher name to `megatron-lm`.
`examples/slurm-configs/basic/09-megatron-lm-multi-node.json`	Updates launcher name and uses `distributed.port`.
`examples/k8s-configs/README.md`	Updates example file layout/paths and clarifies K8s-specific config semantics.
`examples/k8s-configs/minimal/megatron-lm-minimal.json`	Updates launcher name to `megatron-lm`.
`examples/k8s-configs/minimal/megatron-lm-exclude-node.json`	Updates launcher name to `megatron-lm`.
`examples/k8s-configs/minimal/megatron-lm-optimized.json`	Updates launcher name and uses `distributed.port`.
`examples/k8s-configs/basic/02-torchrun-single-node-multi-gpu.json`	Uses `distributed.port` in example config.
`examples/k8s-configs/basic/02-torchrun-single-node-multi-gpu-tools.json`	Uses `distributed.port` in example config.
`examples/k8s-configs/basic/03-torchrun-multi-node-basic.json`	Uses `distributed.port` in example config.
`examples/k8s-configs/basic/04-torchrun-multi-node-advanced.json`	Uses `distributed.port` in example config.
`examples/k8s-configs/basic/05-torchrun-nvidia-gpu-example.json`	Uses `distributed.port` in example config.
`examples/k8s-configs/basic/06-data-provider-with-pvc.json`	Uses `distributed.port` in example config.
`examples/k8s-configs/basic/megatron-lm-multi-node-basic.json`	Uses `distributed.port` in example config.
`examples/k8s-configs/basic/vllm-multi-node-basic.json`	Uses `distributed.port` in example config.
`examples/k8s-configs/basic/torchtitan-multi-node-basic.json`	Uses `distributed.port` in example config.
`examples/k8s-configs/basic/sglang-multi-node-basic.json`	Uses `distributed.port` in example config.
`examples/k8s-configs/basic/sglang-disagg-multi-node-basic.json`	Uses `distributed.port` in example config.
`examples/k8s-configs/basic/sglang-disagg-custom-split.json`	Uses `distributed.port` in example config.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

  "distributed": {
    "enabled": true,
    "backend": "nccl",
    "launcher": "torchrun",
    "nnodes": 2,
    "nproc_per_node": 2,
-    "master_port": 29500
+    "port": 29500
  },


    "launcher": "torchrun",
    "nnodes": 4,
    "nproc_per_node": 2,
-    "master_port": 29500
+    "port": 29500
  },


    "launcher": "torchrun",
    "nnodes": 1,
    "nproc_per_node": 2,
-    "master_port": 29500
+    "port": 29500
  },


    "launcher": "torchrun",
    "nnodes": 1,
    "nproc_per_node": 2,
-    "master_port": 29500
+    "port": 29500
  },


    "launcher": "torchrun",
    "nnodes": 1,
    "nproc_per_node": 4,
-    "master_port": 29500
+    "port": 29500
  },


    "launcher": "sglang",
    "nnodes": 2,
    "nproc_per_node": 4,
-    "master_port": 29500
+    "port": 29500
  },


    "launcher": "sglang-disagg",
    "nnodes": 5,
    "nproc_per_node": 8,
-    "master_port": 29500
+    "port": 29500
  },


    "launcher": "sglang-disagg",
    "nnodes": 7,
    "nproc_per_node": 8,
-    "master_port": 29500,
+    "port": 29500,
    "sglang_disagg": {


  "distributed": {
-    "launcher": "megatron",
+    "launcher": "megatron-lm",
    "nnodes": 4,
    "nproc_per_node": 8,
-    "master_port": 29500
+    "port": 29500
  },


  "distributed": {
    "enabled": true,
    "backend": "nccl",
-    "launcher": "megatron",
+    "launcher": "megatron-lm",
    "nnodes": 1,
    "nproc_per_node": 2,
-    "master_port": 29500
+    "port": 29500
  },


Copilot

Pull request overview

Copilot reviewed 47 out of 49 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/madengine/cli/validators.py:291

In the ConfigurationError path you only print the raw invalid value plus suggestions, but you drop the actual exception message (which includes the source/context like "... in additional_context"). This makes it harder to diagnose which field triggered the failure. Consider printing str(exc) (or including the source in your own message), and pass a more specific source string (e.g. additional_context.distributed.launcher vs additional_context.launcher.type) when calling validate_launcher().
src/madengine/deployment/common.py:55
PR title/description focus on documentation changes, but this diff also introduces significant runtime behavior changes (new launcher validation + canonicalization, deployment template logic changes, and new/updated unit tests). Please update the PR description (and test plan) to reflect these non-doc changes so reviewers know to focus on both documentation and behavior.

…sage Corrects --csv-file to the real --csv-file-path/--file flags, fills in missing `database` command flags (--unique-key/-k, --batch-size, --no-upsert, --no-index, --dry-run, MONGO_AUTH_SOURCE/MONGO_TIMEOUT_MS), fixes wrong `run --output`/`--tools-config` defaults, fixes broken space-separated --tags syntax, and updates the stale version badge.

Removes fabricated timeout_multiplier/service_account keys and the vllm.tensor_parallel_size example (never read; real value derives from distributed.nproc_per_node). Fixes SLURM gpus_per_node default (1 -> 8), docker_env_vars.MAD_ROCM_PATH -> ROCM_PATH, the Data Provider schema (fabricated data_sources wrapper -> real flat shape), and credential key names (AMD_GITHUB -> PUBLIC_GITHUB_ROCM_KEY, uppercase USERNAME/PASSWORD).

Corrects the backwards/incomplete K8s "Configuration Priority" list to match config_loader.py's real 5-layer merge order (including the previously-missing AMD multi-GPU vendor layer), and removes nonexistent SLURM fields (mem, mail_user, mail_type). In examples/k8s-configs and examples/slurm-configs READMEs: fixes ~35 stale/renamed file paths, removes dead Primus example references, marks gpu_vendor/guest_os/deploy as non-required with real defaults, removes the invalid "RHEL" guest_os value, documents results_pvc and host_ipc as non-configurable (host_ipc previously told users to set a key that is always silently overridden), adds missing results_storage_size / data_storage_size / allow_privileged_profiling fields, and documents the SLURM node-health preflight keys (enable_node_check, auto_cleanup_nodes, allow_submit_without_clean_nodes, verbose_node_check).

…ples Fixes "megatron" -> "megatron-lm" launcher value, removes the nonexistent --config flag in favor of --additional-context-file, clarifies the dead distributed.master_port key (SLURM reads distributed.port; K8s reads a separate top-level launcher.master_port object), corrects the 5-node auto-split table row (1/3, not 2/2), fixes the vLLM SLURM multi-node description (data-parallel, not TP+PP with Ray), and removes broken Primus example links.

…ample Removes the nonexistent --tools therock_check flag, fixes the "Default Tool Configuration" example (rocprof command, gpu_info_power_profiler env vars using bare names instead of the real POWER_/VRAM_-prefixed names) across all Multi-GPU and sampling-rate examples, and adds the undocumented tool names (rocprof_hip_only, rocprof_sys, rocprofv3, rocprofv3_agent, rocprofv3_agent_counter, hipblaslt_trace, instruction_mix.txt).

… refs Notes that deployment_config in build_manifest.json is only written for non-local deployments (per _save_deployment_config in build_orchestrator.py), and adds the always-present context/ credentials_required keys to the example. Removes a duplicated `madengine --version` line in installation.md and fixes a stale test path in contributing.md (tests/test_cli.py -> tests/unit/test_cli.py).

The README described the module as "Not yet implemented" and documented a fictional future API (mongodb_client.py/MongoDBClient, local_storage.py/LocalStorage, api.py/ingest_results()), even though mongodb.py is fully implemented and wired into the `database` CLI command. Rewrites the README around the real classes (MongoDBConfig, UploadOptions, UploadResult, DocumentLoader/JSONLoader/CSVLoader, DocumentTransformer, MongoDBUploader, upload_file_to_mongodb) and adds a CLI-flag-to-API-param mapping table.

execution/README.md: fixes build_all_models's models_list -> models param, run_container's fabricated model_docker/gpu_ids params -> real docker_image string param, wrong status value casing/set (successful/failed/timeout -> SUCCESS/FAILURE/SKIPPED), wrong result key (duration -> test_duration), and documents the previously-missing dockerfile_utils.py and container_runner_helpers.py files. reporting/README.md: removes the fabricated "Legacy Reporting Tools" section claiming csv_to_html.py/csv_to_email.py live in a nonexistent tools/ directory and are unused by the modern CLI (they live in reporting/ and back `report to-html`/`report to-email`), fixes the update_perf_csv()/flatten_tags() example signatures, documents the perf_entry.csv/.json side effect, and adds a missing entry for update_perf_super.py.

…eferences - Fix remaining megatron -> megatron-lm launcher references in README, usage, and configuration docs. - Document DOCKER_CONFIG and MAD_SKIP_DOCKER_LOGIN, and clarify that MAD_CONTAINER_IMAGE is an --additional-context key, not an env var (cli-reference, configuration, usage). - Add missing Kubernetes and SLURM additional_context keys (cluster/scheduling, storage, node health/results) with pointers to the example READMEs for full reference. - Correct SGLang Disaggregated minimum node counts and split formula to reflect the SLURM co-located-proxy layout vs. Kubernetes' dedicated proxy requirement. - Replace stale hardcoded version/date footer with a pointer to `madengine --version` and CHANGELOG.md.

Copilot

Pull request overview

Copilot reviewed 16 out of 18 changed files in this pull request and generated no new comments.

coketaste self-assigned this Jul 16, 2026

coketaste changed the title ~~docs: rewrite README as concise landing page and add architecture diagrams [WIP]~~ docs: rewrite README as concise landing page and add architecture diagrams Aug 19, 2026

coketaste marked this pull request as ready for review August 19, 2026 15:48

coketaste requested review from Rohan138 and gargrahul as code owners August 19, 2026 15:48

Copilot AI lite review requested due to automatic review settings August 19, 2026 15:48

coketaste requested review from Cemberk and leconcio as code owners August 19, 2026 15:48

Copilot started reviewing on behalf of coketaste August 19, 2026 15:48 View session

Copilot AI reviewed Aug 19, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings August 20, 2026 00:41

Copilot started reviewing on behalf of coketaste August 20, 2026 00:42 View session

Copilot AI reviewed Aug 20, 2026

View reviewed changes

coketaste added 10 commits August 19, 2026 20:02

Merge branch 'develop' into coketaste/docs-rewrite

69d5f2f

Copilot AI review requested due to automatic review settings August 20, 2026 01:04

coketaste force-pushed the coketaste/docs-rewrite branch from 7567594 to cb17652 Compare August 20, 2026 01:04

Copilot started reviewing on behalf of coketaste August 20, 2026 01:05 View session

Copilot AI reviewed Aug 20, 2026

View reviewed changes

coketaste merged commit 4f281ef into develop Aug 20, 2026
1 check passed

coketaste added a commit that referenced this pull request Aug 20, 2026

docs(changelog): add entry for README rewrite (#161)

6bd9387

Navigation Menu

Uh oh!

docs: rewrite README as concise landing page and add architecture diagrams - #161

docs: rewrite README as concise landing page and add architecture diagrams#161
coketaste merged 11 commits into
developfrom
coketaste/docs-rewrite

coketaste commented Jul 16, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

coketaste commented Jul 16, 2026

Summary

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants