docs: add details in gpu selection#65
docs: add details in gpu selection#65nicolasboisseau wants to merge 3 commits intosnakemake:mainfrom
Conversation
📝 WalkthroughWalkthroughDocs updated in Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 1
🤖 Fix all issues with AI agents
In `@docs/further.md`:
- Around line 114-115: Correct the grammar in the sentence on line 114 by
removing the extra "the" and hyphenating "machine-compatible": change "using the
a machine compatible label" to "using a machine-compatible label" while keeping
the example parameter `nvidia_gpu='nvidia-tesla-v100' unchanged.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/further.md
🧰 Additional context used
🪛 LanguageTool
docs/further.md
[grammar] ~114-~114: Use a hyphen to join words.
Context: ...he gpu type directly using the a machine compatible label: e.g. nvidia_gpu='nvidi...
(QB_NEW_EN_HYPHEN)
🔇 Additional comments (1)
docs/further.md (1)
112-112: Good addition for clarity.This explicitly documents the default GPU type for n1-* machines, which will help users understand the behavior when they specify
gpu_nvidia=1.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@docs/further.md`:
- Around line 110-115: Update the incorrect parameter name used in the
documentation from gpu_nvidia to nvidia_gpu so it matches the codebase; replace
occurrences of "gpu_nvidia=1" with "nvidia_gpu=1" in the example text and ensure
the wording aligns with the usage in executor.py and the hello-world-gpu
Snakefile, preserving the note about machine type and the machine-compatible
label example (nvidia_gpu='nvidia-tesla-v100').
🧹 Nitpick comments (2)
docs/further.md (2)
112-112: Correct grammar, capitalization, and punctuation.Line 112 has several style issues:
- Use "an" instead of "a" before "n1-*" (vowel sound)
- Capitalize "GPU" for consistency with technical documentation standards
- Add period at the end of the sentence
📝 Proposed fix
-On a n1-* family machine type, gpu_nvidia=1 will trigger a "nvidia-tesla-t4" gpu by default. +On an n1-* family machine type, gpu_nvidia=1 will trigger an "nvidia-tesla-t4" GPU by default.
114-114: Capitalize "GPU" for consistency.For consistency with technical documentation standards, "gpu type" should be "GPU type".
📝 Proposed fix
-It's possible to change the gpu type directly using a machine-compatible label: +It's possible to change the GPU type directly using a machine-compatible label:
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/further.md
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@docs/further.md`:
- Line 112: Update the sentence containing "a n1-*" and the `nvidia_gpu=1`
claim: change "a n1-*" to "an n1-*", remove the incorrect assertion that
`nvidia_gpu=1` defaults to "nvidia-tesla-t4", and replace it with a clear
statement that Batch does not default a GPU type when only the GPU count is
set—you must explicitly specify the GPU type or use an accelerator-optimized
machine type; reference the exact tokens `nvidia_gpu=1` and "nvidia-tesla-t4"
when editing so the incorrect claim is removed and the corrected wording is
inserted.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/further.md
🔇 Additional comments (2)
docs/further.md (2)
110-110: LGTM! Parameter name corrected.The documentation now correctly uses
nvidia_gpu=1instead of the previous incorrectgpu_nvidia. This aligns with the codebase usage.
114-115: No changes needed. The documented syntaxnvidia_gpu='nvidia-tesla-v100'is valid and correctly supported by the executor implementation. The codebase confirms that thenvidia_gpuparameter accepts both numeric values (for GPU count, with default type "nvidia-tesla-t4") and string values (for GPU model type specification).
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Added an example of GPU labelling in the documentation to make it more explicit. (It can help snakemake newcomers)
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.