docs: Refactor Home Page and New About Section#1338
Conversation
📝 WalkthroughWalkthroughAdds a new Sphinx documentation build workflow (Makefile) with OS-aware virtualenv handling, multiple build/publish/live targets, and Pinecone-related tasks. Introduces/updates docs content across “About” and “Algorithms” sections, adjusts Sphinx config and dependencies, and updates the docs landing page with standardized captions and an API Reference toctree. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant MK as make
participant UV as uv / .venv-docs
participant SPH as Sphinx
participant LIVE as Live Server
participant PC as Pinecone
Dev->>MK: make docs-env
MK->>UV: Check/install uv, create .venv-docs
UV-->>Dev: Activation instructions
Dev->>MK: make docs-html (DOCS_ENV=ga|ea|internal|draft)
MK->>UV: Run docs python
UV->>SPH: Build HTML (tags, fail-on-warn as configured)
SPH-->>Dev: _build/html
Dev->>MK: make docs-live-<env>
MK->>LIVE: Serve _build/html (auto-reload)
Dev->>MK: make docs-publish-<env>
MK->>UV: Run publish script
UV-->>Dev: Publish result
Dev->>MK: make docs-pinecone-update
MK->>SPH: (depends) Build HTML
MK->>UV: Run Pinecone update script
UV->>PC: Upsert/update index
PC-->>Dev: Update status
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (6)
docs/about/backends.md (1)
7-11: Clarify naming: “Megatron Core” vs “Megatron”.Use “Megatron Core” for the training backend to match repo terminology and reduce ambiguity with the inference backend.
-- [**Megatron**](https://github.com/NVIDIA-NeMo/Megatron-Bridge) - NVIDIA's high-performance training framework for scaling to large models with 6D parallelisms +- [**Megatron Core**](https://github.com/NVIDIA-NeMo/Megatron-Bridge) - NVIDIA's high-performance training framework for scaling to large models with 6D parallelismsdocs/about/algorithms/on-policy-distillation.md (1)
32-43: Use NUM_ACTOR_NODES in COMMAND and template names to avoid drift.Keep node counts consistent between sbatch flags and config overrides.
-COMMAND="uv run ./examples/run_distillation_math.py --config examples/configs/distillation_math.yaml cluster.num_nodes=2 cluster.gpus_per_node=8 checkpointing.checkpoint_dir='results/distill_2nodes' logger.wandb_enabled=True logger.wandb.name='distill-2nodes'" \ +COMMAND="uv run ./examples/run_distillation_math.py --config examples/configs/distillation_math.yaml cluster.num_nodes=${NUM_ACTOR_NODES} cluster.gpus_per_node=8 checkpointing.checkpoint_dir='results/distill_${NUM_ACTOR_NODES}nodes' logger.wandb_enabled=True logger.wandb.name='distill-${NUM_ACTOR_NODES}nodes'" \docs/about/algorithms/rm.md (2)
9-11: Unify command style.Use a single invocation style for clarity (either uv run python examples/run_rm.py or uv run ./examples/run_rm.py) across the page.
Also applies to: 17-19
32-43: Use NUM_ACTOR_NODES consistently.Hard-coded cluster.num_nodes=2; parameterize to match NUM_ACTOR_NODES to avoid drift.
Apply:
-COMMAND="uv run ./examples/run_rm.py --config examples/configs/rm.yaml cluster.num_nodes=2 cluster.gpus_per_node=8 checkpointing.checkpoint_dir='results/rm_llama1b_2nodes' logger.wandb_enabled=True logger.wandb.name='rm-llama1b-2nodes'" \ +COMMAND="uv run ./examples/run_rm.py --config examples/configs/rm.yaml cluster.num_nodes=${NUM_ACTOR_NODES} cluster.gpus_per_node=8 checkpointing.checkpoint_dir='results/rm_llama1b_${NUM_ACTOR_NODES}nodes' logger.wandb_enabled=True logger.wandb.name='rm-llama1b-${NUM_ACTOR_NODES}nodes'" \Makefile (2)
225-240: Ensure env is prepared before Pinecone actions.Add dependency on docs-env so scripts run with installed deps. Also consider passing explicit -- to script args.
Apply:
-docs-pinecone-test: +docs-pinecone-test: docs-env @@ -docs-pinecone-upload-dry: +docs-pinecone-upload-dry: docs-env @@ -docs-pinecone-upload: +docs-pinecone-upload: docs-env @@ -docs-pinecone-update: docs-html +docs-pinecone-update: docs-html
6-10: .PHONY duplication and missing conventional aliases (optional).Deduplicate .PHONY and add conventional all/clean/test aliases to satisfy checkmake and developer expectations.
Apply:
-.PHONY: help docs-html docs-clean docs-live docs-env docs-publish \ +.PHONY: help all clean test docs-html docs-clean docs-live docs-env docs-publish \ docs-html-internal docs-html-ga docs-html-ea docs-html-draft \ docs-live-internal docs-live-ga docs-live-ea docs-live-draft \ docs-publish-internal docs-publish-ga docs-publish-ea docs-publish-draft \ docs-pinecone-test docs-pinecone-upload docs-pinecone-upload-dry docs-pinecone-update @@ -.PHONY: docs-html docs-clean docs-live docs-env +all: docs-html +clean: docs-clean +test: docs-pinecone-test +.PHONY: docs-html docs-clean docs-live docs-envAlso applies to: 88-95
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
Makefile(1 hunks)docs/about/algorithms/dpo.md(1 hunks)docs/about/algorithms/grpo.md(1 hunks)docs/about/algorithms/index.md(1 hunks)docs/about/algorithms/on-policy-distillation.md(1 hunks)docs/about/algorithms/rm.md(1 hunks)docs/about/algorithms/sft.md(1 hunks)docs/about/backends.md(1 hunks)docs/about/clusters.md(1 hunks)docs/about/evaluation.md(1 hunks)docs/about/features.md(1 hunks)docs/about/installation.md(1 hunks)docs/about/overview.md(1 hunks)docs/about/quick-start.md(1 hunks)docs/about/tips-and-tricks.md(1 hunks)docs/conf.py(1 hunks)docs/index.md(5 hunks)pyproject.toml(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
docs/**/*.md
📄 CodeRabbit inference engine (CODING_GUIDELINES.md)
When a markdown doc under docs/**/*.md is added or renamed, update docs/index.md to include it in the appropriate section
Files:
docs/about/overview.mddocs/about/features.mddocs/about/clusters.mddocs/about/algorithms/on-policy-distillation.mddocs/about/algorithms/dpo.mddocs/about/algorithms/index.mddocs/about/algorithms/grpo.mddocs/about/algorithms/rm.mddocs/about/backends.mddocs/index.mddocs/about/algorithms/sft.mddocs/about/tips-and-tricks.mddocs/about/installation.mddocs/about/quick-start.mddocs/about/evaluation.md
**/*.py
📄 CodeRabbit inference engine (CODING_GUIDELINES.md)
**/*.py: Follow the Google Python Style Guide for all Python code
Target Python 3.12+ for all Python code in NeMo-RL
Indent Python code with 4 spaces; do not use tabs
Python filenames should be snake_case (e.g., some_file.py)
Class names should be PascalCase
Function and method names should be snake_case
Local variable names should be snake_case; if starting with a number, prefix with k (e.g., k_99th_percentile)
Global variables should be UPPER_SNAKE_CASE and prefixed with G_ (e.g., G_MY_GLOBAL)
Constants should be UPPER_SNAKE_CASE
Avoid shadowing variables declared in an outer scope
Initialize all externally visible members of a class in the constructor
For public interfaces used outside a file, prefer docstrings over comments
Use comments mainly for code within a function or interfaces local to a file
Commented-out code must include a nearby comment explaining usage and why it is commented out; otherwise remove before merging
Use Google-style docstrings for classes and functions (Sphinx-parseable)
Avoid using reflection when functionality can be easily achieved without it
Limit except clauses to the smallest specific set of exceptions possible
For duck-typing via try/except, keep the try body minimal and use else for main logic
Add the NVIDIA copyright header (with current year) at the top of all Python files, excluding tests/ and test-only scripts
Files:
docs/conf.py
🪛 checkmake (0.2.2)
Makefile
[warning] 13-13: Target body for "help" exceeds allowed length of 5 (37).
(maxbodylength)
[warning] 90-90: Missing required phony target "all"
(minphony)
[warning] 90-90: Missing required phony target "clean"
(minphony)
[warning] 90-90: Missing required phony target "test"
(minphony)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Post automodel integration comment / Comment on PR
- GitHub Check: Post submodule check comment / Comment on PR
🔇 Additional comments (38)
docs/about/clusters.md (2)
1-4: Ensure inclusion in docs index.Add this page to docs/index.md in the appropriate section so it appears in navigation.
3-3: Do not change the link—../cluster.md is correct. There is no cluster-start.md file; the “Cluster Start” guide lives at docs/cluster.md and is already properly referenced via../cluster.md.Likely an incorrect or invalid review comment.
docs/about/installation.md (2)
1-113: No action needed: Installation is already linked in docs/index.mdLikely an incorrect or invalid review comment.
87-91: Link to ../docker.md correctly resolves to docs/docker.md (file exists).docs/about/features.md (1)
1-32: docs/index.md already lists About/Features
No action required.docs/about/tips-and-tricks.md (2)
1-43: No index update needed. ‘about/tips-and-tricks’ is already listed in docs/index.md.
20-21: Fix documentation script reference
Neitherexamples/run_grpo.pynorexamples/run_grpo_math.pyexists. Confirm the intended script name and update this snippet accordingly.docs/about/quick-start.md (1)
1-42: Quick Start content looks good.
MyST notes and tables render fine; links to installation and tips are consistent.docs/about/backends.md (1)
10-10: Verified linked docs exist. Bothdocs/design-docs/training-backends.mdanddocs/design-docs/generation.mdare present, andabout/backendsis indexed indocs/index.md.pyproject.toml (1)
129-138: Scope AI deps out of core docs and verify extensions
- Move “openai”, “pinecone”, “python-dotenv” out of the core docs extras into a separate
docs-aigroup.- Manually confirm that
sphinxcontrib-mermaidandswagger-plugin-for-sphinxsupport Sphinx 8.x and are correctly enabled in yourconf.py(or consider usingsphinxcontrib-openapiif compatibility issues arise).docs/about/evaluation.md (3)
33-56: LGTM!The evaluation commands are well-documented with clear examples and helpful inline comments. The note about evaluation variance is valuable for users.
31-31: Checkpointing documentation link is valid.
Filedocs/design-docs/checkpointing.mdexists and is correctly referenced.
61-61: References verified
Bothexamples/configs/evals/eval.yamlanddocs/guides/eval.mdexist and the links are correct.docs/conf.py (2)
45-45: Clarify the conditional logic for autodoc2.The comment states autodoc2 will be "Added conditionally below based on package availability," but there's no implementation of this conditional logic visible in the provided code. Either implement the conditional check or clarify if this will be addressed in a future change.
Consider adding the conditional logic, for example:
try: import autodoc2 extensions.append("autodoc2") except ImportError: passOr update the comment to indicate this is a TODO item for future implementation.
50-52: Extension dependencies are declared in pyproject.toml.
pyproject.toml includessphinx-design(line 129) andsphinxcontrib-mermaid(line 136);sphinx.ext.ifconfigis built into Sphinx core and needs no external listing.docs/about/overview.md (2)
1-13: LGTM!The overview content is clear, well-structured, and accurately describes the project's capabilities and value proposition.
14-18: Referenced documentation file verified.docs/design-docs/design-and-philosophy.mdexists in the repository.docs/about/algorithms/index.md (2)
15-24: LGTM!The toctree structure is properly configured with appropriate depth and navigation settings. The entries align with the algorithm files referenced in the support matrix.
7-13: All algorithm documentation links valid. Verified that every referenced markdown file and section anchor exists under docs/about/algorithms.docs/about/algorithms/sft.md (4)
9-23: LGTM!The command examples are clear and well-documented. The progression from single-GPU to multi-GPU usage with parameter adjustments is helpful for users.
27-44: LGTM!The multi-node deployment example is well-structured with clear placeholders for customization. The sbatch configuration is appropriate for a Slurm cluster environment.
25-25: Configuration file reference is correct;examples/configs/sft.yamlexists.
3-3: SFT guide link is valid. The filedocs/guides/sft.mdexists at the referenced path.docs/about/algorithms/grpo.md (7)
7-43: LGTM!The single-node examples are comprehensive, showing progression from basic usage to multi-GPU and different backend configurations. The examples are clear and well-commented.
47-65: LGTM!The multi-node deployment example follows the same clear structure as other algorithm examples, making it easy for users familiar with one algorithm to adapt to another.
69-93: LGTM!The large-scale deployment example for Qwen2.5-32B is comprehensive, showing proper environment setup and advanced parallelism configurations. The pre-download recommendation (line 78) is a valuable best practice.
95-101: LGTM!The multi-turn section concisely introduces the capability with a practical example. The sliding puzzle use case effectively demonstrates the feature.
67-67: Docker documentation link is valid
The link../../docker.mdcorrectly resolves todocs/docker.md, which exists.
45-45: Training backends doc link is correct.
The file docs/design-docs/training-backends.md exists and the reference is valid.
5-5: GRPO guide reference is valid.
The filedocs/guides/grpo.mdexists at the referenced location.docs/about/algorithms/dpo.md (4)
1-3: LGTM!The introduction clearly describes the DPO example and references the dataset appropriately.
5-33: LGTM!The DPO single-node examples follow the established pattern from other algorithm documentation, making it easy for users to understand. The parameter customization examples are well-chosen and relevant.
37-57: LGTM!The multi-node deployment example maintains consistency with other algorithm documentation, which helps users who work across multiple algorithms.
35-35: Referenced files verified: bothexamples/configs/dpo.yamlanddocs/guides/dpo.mdexist.docs/index.md (2)
1-167: LGTM!The restructured landing page with grid cards provides excellent user experience and clear navigation to all documentation sections. The use of octicon icons is consistent and helpful.
168-252: Caption standardization improves consistency.The removal of emojis from toctree captions creates a more professional and consistent documentation structure. All new documentation files from this PR are properly included in the appropriate sections.
Based on coding guidelines: "When a markdown doc under docs/**/*.md is added or renamed, update docs/index.md to include it in the appropriate section" - this has been properly done for the new files added in this PR.
As per coding guidelines
docs/about/algorithms/rm.md (2)
21-21: RM guide link is correct
The file atdocs/guides/rm.mdexists and is included indocs/index.md; no changes needed.
1-44: No toctree update needed.
The RM page is already listed in docs/about/algorithms/index.md; no action required.
There was a problem hiding this comment.
thanks jennifer. some comments:
- Can you move the
Makefileintodocs/, I don't think it needs to be visible if not working on the docs - can you adjust the makefile to use
uvlike https://github.com/NVIDIA-NeMo/RL/blob/main/docs/documentation.md. this is to keep the dependencies consistent with what we put in pyproject.toml/uv.lock (i also tried this locally and it didn't work b/c there's a lot of assumptions on the virtual env, this is solved when we useuv) - there's a lot going on in that makefile. is everything necessary? if there's stuff there that is largely unused, can we remove what we don't need? status quo is there are only four commands in https://github.com/NVIDIA-NeMo/RL/blob/main/docs/documentation.md, but now there are lots. it's always easier to add than to take away, so prefer keeping it slim to what we need and add as needed.
- i see you've split up the frontpage readme into different sections. is the steady state to remove the front page readme and just link to the documentation? my preference is to have it only in one place, since editing it in two places is error prone
- can you change all the admonitions to the github style so it looks good even if someone stumbled on the docs from github?
❌ Submodule Fast-Forward Check FailedCheck based on commit: 785d4e6 (PR #1338 from ❌ Submodules that need attention:Automodel: ❌ PR branch is BEHIND main branch Megatron-Bridge: ❌ PR branch is BEHIND main branch Please ensure all submodule commits are fast-forwards of the main branch before merging. |
❌ Submodule Fast-Forward Check FailedCheck based on commit: a887fda (PR #1338 from ❌ Submodules that need attention:Automodel: ❌ PR branch is BEHIND main branch Megatron-Bridge: ❌ PR branch is BEHIND main branch Please ensure all submodule commits are fast-forwards of the main branch before merging. |
Hey @terrykong, Jennifer is out for the next few months and I'll be helping you with your docs.
|
❌ Submodule Fast-Forward Check FailedCheck based on commit: da8a451 (PR #1338 from ❌ Submodules that need attention:Automodel: ❌ PR branch is BEHIND main branch Megatron-Bridge: ❌ PR branch is BEHIND main branch Please ensure all submodule commits are fast-forwards of the main branch before merging. |
4587f81 to
5eb41f9
Compare
Signed-off-by: Terry Kong <terryk@nvidia.com>
Signed-off-by: Lawrence Lane <llane@nvidia.com>
a7a307c to
9c211d8
Compare
Signed-off-by: jgerh <jgerhold@nvidia.com> Signed-off-by: Lawrence Lane <llane@nvidia.com> Signed-off-by: Ashwath Aithal <aaithal@nvidia.com> Signed-off-by: L.B. <llane@nvidia.com> Signed-off-by: Terry Kong <terryk@nvidia.com> Co-authored-by: Ashwath Aithal <aaithal@nvidia.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Lawrence Lane <llane@nvidia.com> Co-authored-by: Wenwen Gao <94138584+snowmanwwg@users.noreply.github.com> Co-authored-by: Terry Kong <terryk@nvidia.com>
Signed-off-by: jgerh <jgerhold@nvidia.com> Signed-off-by: Lawrence Lane <llane@nvidia.com> Signed-off-by: Ashwath Aithal <aaithal@nvidia.com> Signed-off-by: L.B. <llane@nvidia.com> Signed-off-by: Terry Kong <terryk@nvidia.com> Co-authored-by: Ashwath Aithal <aaithal@nvidia.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Lawrence Lane <llane@nvidia.com> Co-authored-by: Wenwen Gao <94138584+snowmanwwg@users.noreply.github.com> Co-authored-by: Terry Kong <terryk@nvidia.com>
Signed-off-by: jgerh <jgerhold@nvidia.com> Signed-off-by: Lawrence Lane <llane@nvidia.com> Signed-off-by: Ashwath Aithal <aaithal@nvidia.com> Signed-off-by: L.B. <llane@nvidia.com> Signed-off-by: Terry Kong <terryk@nvidia.com> Co-authored-by: Ashwath Aithal <aaithal@nvidia.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Lawrence Lane <llane@nvidia.com> Co-authored-by: Wenwen Gao <94138584+snowmanwwg@users.noreply.github.com> Co-authored-by: Terry Kong <terryk@nvidia.com> Signed-off-by: yuanhangs <yuanhangs@nvidia.com>
What does this PR do ?
**Refactor content from README.md file and migrate to new About section.
Summary by CodeRabbit
Documentation
Chores
Style