-
Notifications
You must be signed in to change notification settings - Fork 3
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
Update Documentation #558
Update Documentation #558
Conversation
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
Warning Rate limit exceeded@jan-janssen has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 24 minutes and 38 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughThe changes update executorlib’s documentation and code examples to clarify and distinguish between different executor types. The modifications include renaming and explicitly defining executors as SingleNodeExecutor, HPC Cluster Executor, and HPC Job Executor. Documentation sections, TOC entries, installation instructions, and troubleshooting links are revised accordingly. Additionally, notebook examples and project metadata have been updated to reflect the new nomenclature and organization, ensuring a consistent presentation across the library. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant D as Documentation
participant SN as SingleNodeExecutor
participant HC as HPC Cluster Executor
participant HJ as HPC Job Executor
U->>D: Reads updated docs & examples
D->>SN: Introduces SingleNodeExecutor for local testing
D->>HC: Presents HPC Cluster Executor for cluster submissions
D->>HJ: Explains HPC Job Executor for job-based allocations
U->>SN: Uses SingleNodeExecutor in demonstrations
U->>HC: Adopts HPC Cluster Executor in workflows
U->>HJ: Leverages HPC Job Executor for job management
Poem
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
CodeRabbit Configuration File (
|
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: 1
🧹 Nitpick comments (3)
docs/trouble_shooting.md (1)
41-44
: Clarify HPC Job Executor Description
The description for the special case of the HPC Job Executor is informative. Consider a minor grammatical tweak—possibly inserting a comma after introductory phrases—to improve flow.README.md (1)
139-141
: Minor Duplication in Executor Documentation List
There appears to be repeated wording in the list items for the HPC Job Executor (e.g., “SLURM”, “SLURM with Flux”, and “Flux”). Consider reviewing these for possible redundancy or clarifying the intended distinctions.🧰 Tools
🪛 LanguageTool
[duplication] ~139-~139: Possible typo: you repeated a word.
Context: ...edocs.io/en/latest/3-hpc-job.html) * SLURM * [SLURM with Flux](https://executorlib.readthed...(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~140-~140: Possible typo: you repeated a word.
Context: ...t/3-hpc-job.html#slurm) * SLURM with Flux * [Flux](https://executorlib.readthedocs.io/en/...(ENGLISH_WORD_REPEAT_RULE)
docs/installation.md (1)
58-58
: Specify Language for Fenced Code Blocks
For better markdown compliance and readability, consider specifying a language (e.g.,bash
) for the fenced code blocks in the installation instructions.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
58-58: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
README.md
(6 hunks)docs/_toc.yml
(1 hunks)docs/installation.md
(3 hunks)docs/trouble_shooting.md
(2 hunks)notebooks/1-single-node.ipynb
(12 hunks)notebooks/2-hpc-cluster.ipynb
(11 hunks)notebooks/3-hpc-job.ipynb
(9 hunks)notebooks/4-developer.ipynb
(1 hunks)pyproject.toml
(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- notebooks/4-developer.ipynb
🧰 Additional context used
🪛 LanguageTool
README.md
[typographical] ~41-~41: After the expression ‘for example’ a comma is usually used.
Context: ...le CPU cores, CPU threads or GPUs. For example if the Python function internally uses ...
(COMMA_FOR_EXAMPLE)
[formatting] ~87-~87: Consider inserting a comma after an introductory phrase for better readability.
Context: ..."cores": 2}) print(fs.result()) ``` In this case the [Python simple queuing system adapt...
(IN_THAT_CASE_COMMA)
[duplication] ~139-~139: Possible typo: you repeated a word.
Context: ...edocs.io/en/latest/3-hpc-job.html) * SLURM * [SLURM with Flux](https://executorlib.readthed...
(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~140-~140: Possible typo: you repeated a word.
Context: ...t/3-hpc-job.html#slurm) * SLURM with Flux * [Flux](https://executorlib.readthedocs.io/en/...
(ENGLISH_WORD_REPEAT_RULE)
🪛 markdownlint-cli2 (0.17.2)
docs/installation.md
58-58: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms (12)
- GitHub Check: build (ubuntu-latest, 3.10)
- GitHub Check: build (ubuntu-latest, 3.11)
- GitHub Check: build (ubuntu-latest, 3.12)
- GitHub Check: build (ubuntu-latest, 3.13)
- GitHub Check: build
- GitHub Check: build
- GitHub Check: build (ubuntu-latest, 3.13)
- GitHub Check: build (macos-latest, 3.13)
- GitHub Check: build (windows-latest, 3.13)
- GitHub Check: build (ubuntu-latest, 3.13, .ci_support/environment-mpich.yml)
- GitHub Check: build (ubuntu-latest, 3.13, .ci_support/environment-openmpi.yml)
- GitHub Check: build (macos-latest, 3.13)
🔇 Additional comments (31)
docs/_toc.yml (1)
5-7
: LGTM! The new chapter names better reflect the executor types.The renaming from
local
,hpc-submission
, andhpc-allocation
tosingle-node
,hpc-cluster
, andhpc-job
makes the functionality of each executor type clearer and more intuitive.notebooks/2-hpc-cluster.ipynb (6)
8-9
: LGTM! Clear introduction to HPC Cluster Executor.The introduction clearly explains how HPC Cluster Executor differs from Single Node Executor and HPC Job Executor, particularly in its use of the file system for communication.
31-31
: LGTM! Code examples consistently use the new executor names.The code examples have been updated to use
SlurmClusterExecutor
instead ofSlurmSubmissionExecutor
, maintaining consistency with the new naming scheme.Also applies to: 47-47, 86-86
39-39
: LGTM! Clear explanation of parameter differences.The documentation clearly explains that the only parameter change for
SlurmClusterExecutor
is the cache directory specification.
57-57
: LGTM! Resource dictionary parameters are well documented.The documentation clearly explains how to specify parameters like run time, memory, and submission template through the resource dictionary.
120-120
: LGTM! Dependencies section is clear and accurate.The documentation effectively explains how dependencies are handled using Future objects and communicated to the job scheduler.
222-222
: LGTM! Cache usage is well explained.The documentation clearly explains the importance of cleaning up the cache directory and the use of cloudpickle for serialization.
notebooks/3-hpc-job.ipynb (8)
8-9
: LGTM! Clear introduction to HPC Job Executor.The introduction effectively explains how HPC Job Executor differs from HPC Cluster Executor, particularly in its use of job allocations and zero message queue communication.
12-14
: LGTM! Available functionality is well documented.The documentation clearly lists all available functionality, including parallel execution options and performance optimization features.
56-56
: LGTM! Installation requirements are clearly explained.The documentation effectively explains the importance of selecting a compatible flux version and considering GPU and MPI implementations.
71-71
: LGTM! Resource assignment is well explained.The documentation clearly explains how resource assignment works across different executor types and provides good examples.
127-127
: LGTM! Block allocation is well documented.The documentation effectively explains how block allocation works with the HPC Job Executor.
193-193
: LGTM! Dependencies section is clear and accurate.The documentation effectively explains how dependencies are handled using Future objects from the Python standard library.
235-235
: LGTM! Caching functionality is well explained.The documentation clearly explains how caching works in HPC allocation mode and its similarities to Single Node Executor.
403-403
: LGTM! Flux section is clear and accurate.The documentation effectively explains the current state of Flux adoption and its relationship with SLURM.
notebooks/1-single-node.ipynb (8)
8-9
: LGTM! Clear introduction to Single Node Executor.The introduction effectively explains the purpose of SingleNodeExecutor and its performance characteristics compared to ProcessPoolExecutor.
20-20
: LGTM! Basic functionality is well documented.The documentation clearly explains how to use SingleNodeExecutor as a replacement for ProcessPoolExecutor or ThreadPoolExecutor.
35-35
: LGTM! Usage with with-statement is well explained.The documentation effectively explains the importance of using SingleNodeExecutor with a with-statement to prevent ghost processes.
64-64
: LGTM! Computational efficiency is well explained.The documentation clearly explains when it's efficient to use SingleNodeExecutor for multiple function calls.
124-124
: LGTM! Executor interface compliance is well documented.The documentation effectively explains how SingleNodeExecutor follows the Python standard library's Executor interface.
489-489
: LGTM! Parameter handling is well explained.The documentation clearly explains how parameters from init_function are handled when submitting functions.
635-635
: LGTM! Dependencies section is clear and accurate.The documentation effectively explains how SingleNodeExecutor handles dependencies using Future objects.
686-686
: LGTM! Dependency visualization is well documented.The documentation clearly explains how to visualize dependency graphs using plot_dependency_graph parameter.
pyproject.toml (2)
7-7
: Project Description Update is Clear
The updated description now succinctly emphasizes HPC capabilities, aligning well with the broader project focus.
50-53
: Optional Dependencies Group Renaming
The renaming fromsubmission
tocluster
is consistent with the new nomenclature in the documentation. Please verify that all references (e.g., installation commands) in related files have been updated accordingly.docs/trouble_shooting.md (1)
23-25
: Updated HPC Executor Links
The links for “HPC submission mode” and “HPC allocation mode” now correctly point to the new sections. Ensure that these link changes are harmonized with other documentation files.README.md (3)
26-30
: Refined Executor Links in Introduction
The updated examples now clearly differentiate between the[HPC Cluster Executor](https://executorlib.readthedocs.io/en/latest/2-hpc-cluster.html)
,[HPC Job Executor](https://executorlib.readthedocs.io/en/latest/3-hpc-job.html)
, and[Single Node Executor](https://executorlib.readthedocs.io/en/latest/1-single-node.html)
. This improves clarity on which executor to use in different scenarios.
33-39
: Single Node Executor Example is Spot On
The example usingSingleNodeExecutor()
is clear and demonstrates the basic usage effectively.
69-72
: SLURM Cluster Executor Example
The code snippet illustrating the switch toSlurmClusterExecutor
reads well, and the accompanying explanation emphasizes how executorlib accelerates rapid prototyping.docs/installation.md (2)
54-60
: HPC Cluster Executor Section Update
The section title and installation command (pip install executorlib[cluster]
) now reflect the updated nomenclature. The instructions are clear and align with the changes in other documentation.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
58-58: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
70-76
: HPC Job Executor Section Update
The revised section for HPC Job Executor is comprehensive and consistent with the new terminology. The explanation regarding additional dependencies and the role of the flux framework is well detailed.
README.md
Outdated
with SlurmJobExecutor as exe: | ||
fs = exe.submit(calc, 3, resource_dict={"cores": 2}) | ||
print(fs.result()) | ||
``` |
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.
Instantiation Issue in HPC Job Executor Example
The usage of SlurmJobExecutor
is missing the instantiation parentheses. It should be used as a callable (e.g., SlurmJobExecutor()
) to maintain consistency with the other executor examples.
Apply the following diff:
- with SlurmJobExecutor as exe:
+ with SlurmJobExecutor() as exe:
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
with SlurmJobExecutor as exe: | |
fs = exe.submit(calc, 3, resource_dict={"cores": 2}) | |
print(fs.result()) | |
``` | |
with SlurmJobExecutor() as exe: | |
fs = exe.submit(calc, 3, resource_dict={"cores": 2}) | |
print(fs.result()) |
Summary by CodeRabbit
Documentation
Chore