-
Notifications
You must be signed in to change notification settings - Fork 620
docs: update readme install instructions #2170
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughDocumentation updates were made to clarify and expand installation and development instructions for both the main project and the llama.cpp backend. These changes include additional prerequisite steps, improved formatting, expanded usage examples, and clearer guidance for both CPU and GPU setups. No source code or public API changes were introduced. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 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. 🪧 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
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (5)
README.md (4)
170-174
: Add a language identifier to satisfy markdown-lint and syntax highlightingThe fenced block that starts at line 170 lacks a language spec (MD040).
Addbash
(orsh
) so the linter passes and the snippet is rendered with proper colours.-``` +```bash # Install libnuma apt install -y libnuma-dev(While touching the block you may also prefix the
apt
command withsudo
to avoid permission issues on most systems.)
179-181
: Second fenced block also missing a language specSame MD040 issue for the snippet that shows how to launch the SGLang worker.
-``` +```bash # Note the '.worker' in the module path for SGLang python -m dynamo.sglang.worker --help
210-211
: Mark TensorRT-LLM install snippet as BashLinter again flags the block; add a language tag.
-``` +```bash uv pip install ai-dynamo[trtllm]
281-286
: Trailing whitespace trips the pre-commit hookLines 281-285 contain stray spaces after the backticks and at EOL, causing the
trailing-whitespace
hook to fail.
Delete the extra spaces to unblock CI.-# For development, use␠ +# For development, use export PYTHONPATH="..." -> [!Note]␠ +> [!Note]components/backends/llama_cpp/README.md (1)
4-12
: Fix fenced-block lint error and correct pip option order
- The code fence on line 4 needs a language tag (MD040).
pip install -r --force-reinstall requirements.gpu.txt
has the options reversed;-r
must precede its filename.-``` +# CPU-only install +```bash # Install ai-dynamo llama.cpp backend (CPU Mode) pip install "ai-dynamo[llama_cpp]" # [Optional] To build llama.cpp for CUDA (needs a recent pip) -pip install -r --force-reinstall requirements.gpu.txt +pip install --force-reinstall -r requirements.gpu.txt + python -m dynamo.llama_cpp --model-path /data/models/Qwen3-0.6B-Q8_0.gguf [args]Removing the trailing spaces in lines 5-10 will also clear the
trailing-whitespace
pre-commit error.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
README.md
(3 hunks)components/backends/llama_cpp/README.md
(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
components/backends/llama_cpp/README.md (1)
Learnt from: ptarasiewiczNV
PR: #2027
File: container/deps/vllm/install_vllm.sh:0-0
Timestamp: 2025-07-22T10:22:28.972Z
Learning: The --torch-backend=auto
flag works with vLLM installations via uv pip install, even though it's not a standard pip option. This flag is processed by vLLM's build system during installation to automatically match PyTorch distribution with container CUDA versions.
README.md (2)
Learnt from: fsaady
PR: #1730
File: examples/sglang/slurm_jobs/scripts/worker_setup.py:230-244
Timestamp: 2025-07-03T10:14:30.570Z
Learning: In examples/sglang/slurm_jobs/scripts/worker_setup.py, background processes (like nats-server, etcd) are intentionally left running even if later processes fail. This design choice allows users to manually connect to nodes and debug issues without having to restart the entire SLURM job from scratch, providing operational flexibility for troubleshooting in cluster environments.
Learnt from: nnshah1
PR: #1444
File: tests/fault_tolerance/utils/metrics.py:30-32
Timestamp: 2025-07-01T13:55:03.940Z
Learning: The @dynamo_worker()
decorator in the dynamo codebase returns a wrapper that automatically injects the runtime
parameter before calling the wrapped function. This means callers only need to provide the non-runtime parameters, while the decorator handles injecting the runtime argument automatically. For example, a function with signature async def get_metrics(runtime, log_dir)
decorated with @dynamo_worker()
can be called as get_metrics(log_dir)
because the decorator wrapper injects the runtime parameter.
🪛 markdownlint-cli2 (0.17.2)
components/backends/llama_cpp/README.md
4-4: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
README.md
178-178: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 GitHub Actions: Pre Merge Validation of (ai-dynamo/dynamo/refs/pull/2170/merge) by nv-anants.
components/backends/llama_cpp/README.md
[error] 3-7: pre-commit hook 'trailing-whitespace' failed. Trailing whitespace was fixed in this file. Run 'pre-commit run --all-files' locally to reproduce.
README.md
[error] 3-7: pre-commit hook 'trailing-whitespace' failed. Trailing whitespace was fixed in this file. Run 'pre-commit run --all-files' locally to reproduce.
⏰ 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). (5)
- GitHub Check: pre-merge-rust (lib/runtime/examples)
- GitHub Check: pre-merge-rust (lib/bindings/python)
- GitHub Check: pre-merge-rust (.)
- GitHub Check: Mirror Repository to GitLab
- GitHub Check: Build and Test - vllm
LGTM, thanks for making these edits |
Overview:
minor fixes to READMEs
closes: OPS-514
Summary by CodeRabbit