docs: fix headings in further.md #168
Conversation
WalkthroughThe pull request involves significant modifications to the documentation for the SLURM executor plugin in Snakemake. Changes include reformatting section headers, expanding content for clarity, and introducing new sections that provide detailed instructions on various aspects of SLURM usage. Key areas enhanced include resource specifications, job configurations, and handling job failures. The document aims to improve usability and understanding for users working with SLURM in Snakemake workflows. Changes
Possibly related PRs
Suggested reviewers
Warning Tool Failures:Tool Failure Count:Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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
🧹 Outside diff range and nitpick comments (5)
docs/further.md (5)
404-404: Remove trailing colon from Summary headerThe heading "Summary:" contains a trailing colon which violates markdown best practices. Consider removing it to maintain consistency with other headers.
-### Summary: +### Summary🧰 Tools
🪛 Markdownlint
404-404: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
Line range hint
278-278: Remove development noteThe line contains a development note that should not be present in the final documentation:
-==This is ongoing development. Eventually you will be able to annotate different file access patterns.== +<!-- TODO: Document file access pattern annotation once implemented -->
Line range hint
307-385: Remove duplicate sectionsThe sections "Retries" and "Dynamic Parameterization" appear twice in the document. The duplicate sections starting at line 307 should be removed to avoid confusion and maintain document clarity.
Line range hint
62-89: Enhance MPI example with additional contextThe MPI example would benefit from additional comments explaining:
- Why
srunis recommended over other MPI starters- The significance of the
tasksresource parameter- The relationship between
tasksandcpus_per_taskin hybrid applicationsConsider adding comments to the code example:
rule calc_pi: output: "pi.calc", log: "logs/calc_pi.log", resources: # Number of MPI ranks to launch tasks=10, # Use srun for SLURM-optimized MPI launch mpi="srun", shell: # {resources.mpi} will be replaced with srun "{resources.mpi} -n {resources.tasks} calc-pi-mpi > {output} 2> {log}"
1-1: Consider adding a table of contentsThe document would benefit from a table of contents at the beginning to help readers navigate through the various sections. This is especially important given the length and depth of the documentation.
Example:
### Table of Contents 1. [The General Idea](#the-general-idea) 2. [Specifying Account and Partition](#specifying-account-and-partition) 3. [Ordinary SMP Jobs](#ordinary-smp-jobs) 4. [MPI Jobs](#mpi-jobs) ...
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
docs/further.md(5 hunks)
🧰 Additional context used
🪛 Markdownlint
docs/further.md
404-404: Punctuation: ':'
Trailing punctuation in heading
(MD026, no-trailing-punctuation)
|
@dlaehnemann may I ask you to exercise the 4 eyes principle? |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
docs/further.md (3)
314-314: Fix typo in dynamic parameterization section.The word "different" is repeated.
-Using dynamic parameterization we can react on different different inputs and prevent our HPC jobs from failing. +Using dynamic parameterization we can react on different inputs and prevent our HPC jobs from failing.🧰 Tools
🪛 LanguageTool
[duplication] ~314-~314: Possible typo: you repeated a word
Context: ...ynamic parameterization we can react on different different inputs and prevent our HPC jobs from fa...(ENGLISH_WORD_REPEAT_RULE)
282-282: Remove development note from documentation.Development notes should be tracked in issues or development documentation rather than in user-facing documentation.
-==This is ongoing development. Eventually you will be able to annotate different file access patterns.== +Note: Future versions will support annotation of different file access patterns.
354-354: Remove trailing punctuation from heading.According to Markdown best practices, headings should not end with punctuation marks.
-### Summary: +### Summary🧰 Tools
🪛 Markdownlint
354-354: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
docs/further.md(14 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/further.md
[duplication] ~314-~314: Possible typo: you repeated a word
Context: ...ynamic parameterization we can react on different different inputs and prevent our HPC jobs from fa...
(ENGLISH_WORD_REPEAT_RULE)
🪛 Markdownlint
docs/further.md
354-354: Punctuation: ':'
Trailing punctuation in heading
(MD026, no-trailing-punctuation)
🔇 Additional comments (3)
docs/further.md (3)
1-4: LGTM: Improved document structure and clarity.
The header restructuring and expanded content provide better organization and clearer context for users.
Line range hint 101-160: LGTM: Comprehensive resource specifications documentation.
The resource specifications table and examples provide clear guidance for mapping Snakemake resources to SLURM parameters.
Line range hint 1-363: LGTM: Overall documentation improvements.
The documentation has been significantly improved with:
- Better hierarchical structure
- Comprehensive coverage of SLURM integration
- Clear examples and use cases
- Detailed resource specifications
🧰 Tools
🪛 Markdownlint
354-354: Punctuation: ':'
Trailing punctuation in heading
(MD026, no-trailing-punctuation)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
docs/further.md (4)
354-354: Remove trailing colon from headingAccording to Markdown best practices, headings should not end with punctuation.
-### Summary: +### Summary🧰 Tools
🪛 Markdownlint
354-354: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
281-281: Remove development note from documentationThe line enclosed in "==" markers appears to be a development note that should be removed from the final documentation.
-==This is ongoing development. Eventually you will be able to annotate different file access patterns.==
173-173: Add missing comma for better readabilityAdd a comma after "scheduling" to improve sentence structure.
-For reasons of scheduling multicluster support is provided by the `clusters` flag +For reasons of scheduling, multicluster support is provided by the `clusters` flag🧰 Tools
🪛 LanguageTool
[uncategorized] ~173-~173: Possible missing comma found.
Context: ...## Multicluster Support For reasons of scheduling multicluster support is provided by the...(AI_HYDRA_LEO_MISSING_COMMA)
314-314: Fix word repetitionRemove the repeated word "different".
-Using dynamic parameterization we can react on different different inputs +Using dynamic parameterization we can react on different inputs🧰 Tools
🪛 LanguageTool
[duplication] ~314-~314: Possible typo: you repeated a word
Context: ...ynamic parameterization we can react on different different inputs and prevent our HPC jobs from fa...(ENGLISH_WORD_REPEAT_RULE)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
docs/further.md(14 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/further.md
[uncategorized] ~173-~173: Possible missing comma found.
Context: ...## Multicluster Support For reasons of scheduling multicluster support is provided by the...
(AI_HYDRA_LEO_MISSING_COMMA)
[duplication] ~314-~314: Possible typo: you repeated a word
Context: ...ynamic parameterization we can react on different different inputs and prevent our HPC jobs from fa...
(ENGLISH_WORD_REPEAT_RULE)
🪛 Markdownlint
docs/further.md
354-354: Punctuation: ':'
Trailing punctuation in heading
(MD026, no-trailing-punctuation)
🔇 Additional comments (1)
docs/further.md (1)
360-363: Command line example needs correction
The command example still contains the issues identified in the previous review:
- Duplicate
--workflow-profileparameter - Typo in the parameter name
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (4)
docs/further.md (4)
354-354: Remove trailing colon from heading.For consistency with other headings in the document, remove the trailing colon.
-### Summary: +### Summary🧰 Tools
🪛 Markdownlint
354-354: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
314-314: Fix grammatical issues in dynamic parameterization section.The sentence has a repeated word and incorrect preposition usage.
-Using dynamic parameterization we can react on different different inputs and prevent our HPC jobs from failing. +Using dynamic parameterization we can react to different inputs and prevent our HPC jobs from failing.🧰 Tools
🪛 LanguageTool
[uncategorized] ~314-~314: The preposition “to” seems more likely in this position.
Context: ...g dynamic parameterization we can react on different different inputs and prevent ...(AI_EN_LECTOR_REPLACEMENT_PREPOSITION)
[duplication] ~314-~314: Possible typo: you repeated a word
Context: ...ynamic parameterization we can react on different different inputs and prevent our HPC jobs from fa...(ENGLISH_WORD_REPEAT_RULE)
282-282: Remove development note from documentation.The line contains a development note that should be removed or converted to proper documentation.
-==This is ongoing development. Eventually you will be able to annotate different file access patterns.== +Note: Future versions will support annotation of different file access patterns.
Line range hint
1-363: Consider restructuring advanced topics.The documentation is comprehensive but could benefit from better organization of advanced topics:
- Consider grouping related sections:
- Move "Dynamic Parameterization" near "Advanced Resource Specifications"
- Group "Retries" and "Nesting Jobs" under a "Troubleshooting" section
- Consider adding a table of contents at the beginning for better navigation
🧰 Tools
🪛 Markdownlint
354-354: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
docs/further.md(14 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/further.md
[uncategorized] ~173-~173: A comma might be missing here.
Context: ...## Multicluster Support For reasons of scheduling multicluster support is provided by the...
(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[uncategorized] ~173-~173: You might be missing the article “the” here.
Context: ...t is provided by the clusters flag in resources sections. Note, that you have to write ...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~314-~314: The preposition “to” seems more likely in this position.
Context: ...g dynamic parameterization we can react on different different inputs and prevent ...
(AI_EN_LECTOR_REPLACEMENT_PREPOSITION)
[duplication] ~314-~314: Possible typo: you repeated a word
Context: ...ynamic parameterization we can react on different different inputs and prevent our HPC jobs from fa...
(ENGLISH_WORD_REPEAT_RULE)
🪛 Markdownlint
docs/further.md
354-354: Punctuation: ':'
Trailing punctuation in heading
(MD026, no-trailing-punctuation)
| $ snakemake --workflow-profile <path> \ | ||
| > -j unlimited \ # assuming an unlimited number of jobs | ||
| > --default-resources slurm_account=<account> slurm_partition=<default partition> \ | ||
| > --workflow-profile <profile directory with a `config.yaml`> | ||
| > --configfile config/config.yaml \ | ||
| > --directory <path> # assuming a data path not relative to the workflow | ||
| > --directory <path> # assuming a data path on a different file system than the workflow |
There was a problem hiding this comment.
💡 Codebase verification
Remove duplicate --workflow-profile parameter
The command example in docs/further.md shows --workflow-profile being used twice with different descriptions (<path> and <profile directory with a config.yaml>), which is redundant and confusing. Only one instance of this parameter should be used since they serve the same purpose.
🔗 Analysis chain
Verify command line example consistency.
The command example still shows --workflow-profile being used twice. If this is intentional, consider adding a comment explaining why both parameters are needed. Otherwise, one of them should be removed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if there are other examples in the documentation using --workflow-profile
rg --type md "workflow-profile"
Length of output: 277
Summary by CodeRabbit