docs: add image to front page readme #1320
Conversation
|
👋 Hi faradawn! Thank you for contributing to ai-dynamo/dynamo. Just a reminder: The 🚀 |
github-actions
bot
added
external-contribution
WalkthroughThe README.md file was updated by replacing the top-level heading with a banner image and adding a new section titled "The Era of Multi-Node, Multi-GPU" featuring images and explanatory text about GPU scaling challenges. An "Introducing NVIDIA Dynamo" heading and architecture diagram were also added, along with minor formatting changes. No code or technical instructions were altered. Changes
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
README.md (4)
28-28: Section heading style consistency
### The Era of Multi-Node, Multi-GPUis valid Markdown, but markdownlint flagged a preference for closed ATX headings (### Title ###). Confirm your repository’s heading style guide and update if needed.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
28-28: Heading style
Expected: atx_closed; Actual: atx(MD003, heading-style)
33-33: Refine wording for precision
Consider avoiding the overused word “exactly.” For example:- This orchestration gap is exactly what NVIDIA Dynamo is built to close. + This orchestration gap is precisely what NVIDIA Dynamo is built to close.🧰 Tools
🪛 LanguageTool
[style] ~33-~33: Consider an alternative for the overused word “exactly”.
Context: ... accelerator? This orchestration gap is exactly what NVIDIA Dynamo is built to close. ...(EXACTLY_PRECISELY)
35-35: Fix typo in alt text
The alt text reads “Multi Node Nulti GPU” but should be “Multi Node Multi GPU” for clarity and accessibility:-  + 
39-39: Introducing section heading style
The new### Introducing NVIDIA Dynamoheading follows the same style as above. If you choose closed ATX (### Title ###), update this as well; otherwise it’s fine as-is.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
39-39: Heading style
Expected: atx_closed; Actual: atx(MD003, heading-style)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
docs/images/frontpage-architecture.pngis excluded by!**/*.pngdocs/images/frontpage-banner.pngis excluded by!**/*.pngdocs/images/frontpage-gpu-evolution.pngis excluded by!**/*.pngdocs/images/frontpage-gpu-vertical.pngis excluded by!**/*.png
📒 Files selected for processing (1)
README.md(3 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[style] ~33-~33: Consider an alternative for the overused word “exactly”.
Context: ... accelerator? This orchestration gap is exactly what NVIDIA Dynamo is built to close. ...
(EXACTLY_PRECISELY)
🪛 markdownlint-cli2 (0.17.2)
README.md
28-28: Heading style
Expected: atx_closed; Actual: atx
(MD003, heading-style)
39-39: Heading style
Expected: atx_closed; Actual: atx
(MD003, heading-style)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build and Test - vllm
🔇 Additional comments (3)
README.md (3)
17-17: Add banner image for front page
The banner provides a welcoming visual introduction. Ensure the relative path./docs/images/frontpage-banner.pngresolves correctly on GitHub (the leading./is valid but optional).
30-30: GPU evolution illustration added
Great visual to highlight the memory/compute growth challenge. The alt text “GPU Evolution” is concise and descriptive.
43-43: Add architecture overview diagram
The diagram enriches the README. Current alt text “Dynamo architecture” is sufficient, though you might consider “Architecture diagram of NVIDIA Dynamo” for extra descriptiveness.
|
This looks really great! I'll leave it to @nnshah1 to review the images. Maybe we can use some from some of our official slides. |
|
Thank you @ishandhanani for the quick review! Let me know if there is anything I can add. |
|
Hi @nnshah1 , I updated the branch and plan to merge. Let me know if any issue comes up and I can keep updating the guide, |
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
README.md (1)
17-43: 💡 Verification agent🧩 Analysis chain
Verify image paths and file sizes
Ensure all new images exist under
docs/images/and are reasonably optimized for web (e.g., compressed PNGs under 200 KB).
🏁 Script executed:
#!/bin/bash # List the added frontpage images and their sizes fd -e png docs/images/frontpage-*.png --exec ls -lh {}Length of output: 472
We need to correctly locate and list those frontpage images. Let’s try this instead:
🏁 Script executed:
#!/bin/bash # List all frontpage PNGs under docs/images and show their sizes fd --type f -e png 'frontpage-.*\.png' docs/images --exec ls -lh {} \;Length of output: 420
Optimize frontpage-banner.png size
All frontpage images exist under
docs/images/. Three of them meet the <200 KB guideline:
docs/images/frontpage-architecture.png– 89 KBdocs/images/frontpage-gpu-vertical.png– 151 KBdocs/images/frontpage-gpu-evolution.png– 86 KBHowever,
docs/images/frontpage-banner.pngis 1.7 MB (>200 KB). Please compress or convert it to a more web-friendly format to improve load times.🧰 Tools
🪛 LanguageTool
[style] ~33-~33: Consider an alternative for the overused word “exactly”.
Context: ... accelerator? This orchestration gap is exactly what NVIDIA Dynamo is built to close. ...(EXACTLY_PRECISELY)
[uncategorized] ~41-~41: Possible missing comma found.
Context: ... NVIDIA Dynamo NVIDIA Dynamo is a high-throughput low-latency inference framework designe...(AI_HYDRA_LEO_MISSING_COMMA)
🪛 markdownlint-cli2 (0.17.2)
25-25: Spaces inside link text
null(MD039, no-space-in-links)
28-28: Heading style
Expected: atx_closed; Actual: atx(MD003, heading-style)
39-39: Heading style
Expected: atx_closed; Actual: atx(MD003, heading-style)
🧹 Nitpick comments (3)
README.md (3)
17-17: Add more descriptive alt text for the banner imageThe current alt text
"Dynamo banner"is very generic. For accessibility, consider something like:
28-30: Consolidate or prune redundant GPU visualsYou’ve introduced two GPU-focused graphics (
GPU EvolutionandMulti Node GPU) back-to-back. To reduce visual redundancy and sharpen your narrative, consider removing or merging one of them.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
28-28: Heading style
Expected: atx_closed; Actual: atx(MD003, heading-style)
39-43: Improve alt text for the architecture diagramThe alt text
"Dynamo architecture"is minimal. For clarity, consider a more descriptive caption such as:🧰 Tools
🪛 LanguageTool
[uncategorized] ~41-~41: Possible missing comma found.
Context: ... NVIDIA Dynamo NVIDIA Dynamo is a high-throughput low-latency inference framework designe...(AI_HYDRA_LEO_MISSING_COMMA)
🪛 markdownlint-cli2 (0.17.2)
39-39: Heading style
Expected: atx_closed; Actual: atx(MD003, heading-style)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md(3 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[style] ~33-~33: Consider an alternative for the overused word “exactly”.
Context: ... accelerator? This orchestration gap is exactly what NVIDIA Dynamo is built to close. ...
(EXACTLY_PRECISELY)
[uncategorized] ~41-~41: Possible missing comma found.
Context: ... NVIDIA Dynamo NVIDIA Dynamo is a high-throughput low-latency inference framework designe...
(AI_HYDRA_LEO_MISSING_COMMA)
🪛 markdownlint-cli2 (0.17.2)
README.md
28-28: Heading style
Expected: atx_closed; Actual: atx
(MD003, heading-style)
39-39: Heading style
Expected: atx_closed; Actual: atx
(MD003, heading-style)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build and Test - vllm
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Faradawn Yang <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
README.md (4)
17-17: Improve image alt text for accessibilityThe alt text
"Dynamo banner"is quite generic. Consider making it more descriptive (e.g.,"NVIDIA Dynamo frontpage banner") to better support screen reader users.
28-28: Align ATX heading style
### The Era of Multi-Node, Multi-GPUcurrently lacks closing hashes and triggers MD003. You can either close it as### The Era of Multi-Node, Multi-GPU ###or adjust the linter rules to accept unclosed headings.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
28-28: Heading style
Expected: atx_closed; Actual: atx(MD003, heading-style)
30-30: Enhance alt text for clarityThe alt text
"GPU Evolution"is a bit terse. A more descriptive alt text—such as"Diagram showing GPU evolution across generations"—would improve context for assistive technologies.
43-43: Enrich architecture diagram alt textThe alt text
"Dynamo architecture"could be more illustrative. For example,"Diagram of NVIDIA Dynamo multi-node architecture"would give users better context.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md(3 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[style] ~33-~33: Consider an alternative for the overused word “exactly”.
Context: ... accelerator? This orchestration gap is exactly what NVIDIA Dynamo is built to close. ...
(EXACTLY_PRECISELY)
[uncategorized] ~41-~41: Possible missing comma found.
Context: ... NVIDIA Dynamo NVIDIA Dynamo is a high-throughput low-latency inference framework designe...
(AI_HYDRA_LEO_MISSING_COMMA)
🪛 markdownlint-cli2 (0.17.2)
README.md
28-28: Heading style
Expected: atx_closed; Actual: atx
(MD003, heading-style)
39-39: Heading style
Expected: atx_closed; Actual: atx
(MD003, heading-style)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build and Test - vllm
🔇 Additional comments (1)
README.md (1)
35-35: LGTMThe alt text
"Multi Node Multi-GPU topology"and image path are clear and correctly formatted.
|
Hi @ishandhanani I sync'ed to the main branch and all the images render correctly. Could you help merge, as I couldn't find the merge button. Thanks! |
|
I think we should add. Want me to approve @ishandhanani ? |
|
Thank you @harryskim for checking and merging! Love to keep improving the readme and docs |
3 participants
Overview:
Adds 4 images to the front page. Let me know which one to remove or edit.
Details:
Where should the reviewer start?
Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Summary by CodeRabbit