-
Notifications
You must be signed in to change notification settings - Fork 17
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
docs: requeue #153
base: main
Are you sure you want to change the base?
docs: requeue #153
Conversation
WalkthroughThe pull request introduces new command line flags for the SLURM executor plugin, specifically Changes
Possibly related PRs
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
|
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
🧹 Outside diff range and nitpick comments (4)
docs/further.md (4)
163-170
: Minor improvements needed in the new command line flags section.The new section on additional command line flags is informative and well-structured. However, there are a couple of minor issues to address:
- On line 169, "usefull" should be corrected to "useful".
- The sentence on line 165 could be improved for clarity.
Consider applying these changes:
-This plugin defines additional command line flags. As always the can be used on the command line or in a profile. +This plugin defines additional command line flags. As always, they can be used on the command line or in a profile. -| `--slurm_init_seconds_before_status_checks`| will modify the default time (40 seconds) before the initial status check - usefull for development purposes| +| `--slurm_init_seconds_before_status_checks`| will modify the default time (40 seconds) before the initial status check - useful for development purposes|🧰 Tools
🪛 LanguageTool
[typographical] ~165-~165: Consider adding a comma here.
Context: ...fines additional command line flags. As always the can be used on the command line or ...(AS_ALWAYS_COMMA)
[grammar] ~165-~165: Did you mean “they”, “that” or “there”?
Context: ...dditional command line flags. As always the can be used on the command line or in a...(THE_THEY)
Line range hint
215-283
: Approve changes to "Using Profiles" section with a minor suggestion.The expanded "Using Profiles" section provides valuable information about configuration options, storage prefixes, and file system usage. The examples for different cluster setups are particularly helpful.
Consider formatting the note about ongoing development differently for better visibility:
-==This is ongoing development. Eventually you will be able to annotate different file access patterns.== +> **Note:** This is ongoing development. Eventually, you will be able to annotate different file access patterns.This change will make the note stand out more clearly as a separate piece of information.
🧰 Tools
🪛 LanguageTool
[style] ~215-~215: ‘prior to’ might be wordy. Consider a shorter alternative.
Context: ... amodule load bio VinaLC
immediately prior to execution. Note, that - environment ...(EN_WORDINESS_PREMIUM_PRIOR_TO)
Line range hint
283-308
: Approve changes to "Retries" section with a minor formatting suggestion.The expanded "Retries" section now provides comprehensive information about handling job failures, requeuing, and resuming workflows. The addition of code examples for checking cluster requeue settings and using the
--slurm-requeue
flag is particularly helpful.For consistency with the rest of the document, consider using backticks for the
--rerun-incomplete
flag in the code example:-snakemake ... --rerun-incomplete +snakemake ... `--rerun-incomplete`This change will maintain a consistent style for command-line flags throughout the documentation.
Line range hint
309-370
: Approve changes to "Dynamic Parameterization" section with a minor wording suggestion.The expanded "Dynamic Parameterization" section now provides excellent guidance on dynamically adjusting job parameters, including memory requirements and runtime. The code examples for both Snakefile and workflow profile configurations are clear and illustrative.
Consider rewording the sentence about using sensible settings for improved clarity:
-Be sure to use sensible settings for your cluster and make use of parallel execution (e.g. threads) and [global profiles](#using-profiles) to avoid I/O contention. +Be sure to use appropriate settings for your cluster, utilize parallel execution (e.g., threads), and implement [global profiles](#using-profiles) to optimize performance and avoid I/O contention.This change provides a more specific and actionable recommendation for users.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- docs/further.md (4 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/further.md
[typographical] ~165-~165: Consider adding a comma here.
Context: ...fines additional command line flags. As always the can be used on the command line or ...(AS_ALWAYS_COMMA)
[grammar] ~165-~165: Did you mean “they”, “that” or “there”?
Context: ...dditional command line flags. As always the can be used on the command line or in a...(THE_THEY)
[style] ~215-~215: ‘prior to’ might be wordy. Consider a shorter alternative.
Context: ... amodule load bio VinaLC
immediately prior to execution. Note, that - environment ...(EN_WORDINESS_PREMIUM_PRIOR_TO)
🔇 Additional comments (1)
docs/further.md (1)
Line range hint
371-383
: Approve the "Summary" section.The "Summary" section effectively concludes the document by providing a concise and well-commented example of a typical command line for using the SLURM executor plugin. It successfully ties together the various concepts and options discussed throughout the documentation.
The command line example is clear, comprehensive, and includes helpful comments, making it an excellent reference for users.
🧰 Tools
🪛 LanguageTool
[typographical] ~165-~165: Consider adding a comma here.
Context: ...fines additional command line flags. As always the can be used on the command line or ...(AS_ALWAYS_COMMA)
[grammar] ~165-~165: Did you mean “they”, “that” or “there”?
Context: ...dditional command line flags. As always the can be used on the command line or in a...(THE_THEY)
[uncategorized] ~174-~174: Possible missing comma found.
Context: ...## Multicluster Support For reasons of scheduling multicluster support is provided by the...(AI_HYDRA_LEO_MISSING_COMMA)
only documenting what is new in v0.11.0 (see #152 )
Summary by CodeRabbit
--slurm_init_seconds_before_status_checks
and--slurm_requeue
.--retries
and--slurm-requeue
.