add "JH_UniScheduler" batch_type #459
Conversation
JH UniScheduler was developed by JHINNO company. JH UniScheduler is a commercial software and uses "jsub" to submit tasks. Its overall architecture is similar to that of IBM's LSF. However, there are still some differences between them.
WalkthroughWalkthroughThe update enhances the DPDispatcher Python package with support for a new scheduler system, JH UniScheduler. This includes documentation updates, the addition of a new class for JH UniScheduler operations, and relevant test cases. These changes broaden the package's capabilities in generating and managing HPC job scripts across different scheduler systems, improving its versatility and usability. Changes
Recent Review DetailsConfiguration used: CodeRabbit UI Files selected for processing (1)
Files skipped from review as they are similar to previous changes (1)
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
for more information, see https://pre-commit.ci
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Outside diff range and nitpick comments (2)
tests/context.py (1)
Line range hint
8-34: Consider sorting and formatting the import block for better readability and maintenance.- from dpdispatcher.machines.JH_UniScheduler import JH_UniScheduler # noqa: F401 + from dpdispatcher.machines.JH_UniScheduler import JH_UniSchedulertests/test_class_machine_dispatch.py (1)
Line range hint
8-26: Consider sorting and formatting the import block for better readability and maintenance.- from .context import ( - LSF, - PBS, - BaseContext, - DistributedShell, - LazyLocalContext, - Lebesgue, - LocalContext, - Machine, - Shell, - Slurm, - JH_UniScheduler, - setUpModule, # noqa: F401 - ) + from .context import BaseContext, DistributedShell, JH_UniScheduler, LSF, LazyLocalContext, Lebesgue, LocalContext, Machine, PBS, Shell, Slurm, setUpModule
| import json | ||
| import os | ||
| import sys |
There was a problem hiding this comment.
Tip
Codebase Verification
The script output indicates that all the imports in tests/devel_test_JH_UniScheduler.py are unused. This includes:
jsonossysdpdispatcher.machine.Machinedpdispatcher.submission.Resourcesdpdispatcher.submission.Submissiondpdispatcher.submission.Task
These imports should be reviewed and removed if they are indeed not used in the code.
- Unused import:
import json - Unused import:
import os - Unused import:
import sys - Unused import:
from dpdispatcher.machine import Machine - Unused import:
from dpdispatcher.submission import Resources, Submission, Task
Analysis chain
Ensure all necessary imports are used and there are no unused imports.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for unused imports in the file.
# Test: Search for unused imports. Expect: No output if all imports are used.
rg --type python -e "import" -e "from" tests/devel_test_JH_UniScheduler.py | xargs -I {} ast-grep --lang python --pattern 'import $_' {}
Length of output: 169
Script:
#!/bin/bash
# Description: Check for unused imports in the file.
# List all import statements in the file
imports=$(rg --type py -o -N '^(import|from) .+' tests/devel_test_JH_UniScheduler.py)
# Check each import to see if it is used in the file
for import in $imports; do
# Extract the module name from the import statement
module=$(echo $import | sed -E 's/(import|from) ([^ ]+).*/\2/')
# Search for usage of the module in the file
usage=$(rg --type py -q $module tests/devel_test_JH_UniScheduler.py)
# If the module is not used, print the import statement
if [ -z "$usage" ]; then
echo "Unused import: $import"
fi
done
Length of output: 4165
tests/devel_test_JH_UniScheduler.py
Outdated
| with open("jsons/machine_JH_UniScheduler.json") as f: | ||
| mdata = json.load(f) | ||
|
|
There was a problem hiding this comment.
Tip
Codebase Verification
The directory jsons does not exist, which means the file jsons/machine_JH_UniScheduler.json cannot be accessed. Please verify the correct path or ensure that the directory and file are present in the repository.
Analysis chain
Verify the JSON file path and ensure it is correctly formatted and accessible.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the JSON file path and check its format.
# Test: Check if the JSON file exists and is valid.
if [ -f "jsons/machine_JH_UniScheduler.json" ]; then
echo "JSON file exists."
if jq empty jsons/machine_JH_UniScheduler.json; then
echo "JSON is well formatted."
else
echo "JSON is not well formatted."
fi
else
echo "JSON file does not exist."
fi
Length of output: 108
Script:
#!/bin/bash
# Description: Search for references to "machine_JH_UniScheduler.json" in the codebase to confirm the correct path.
# Search for the JSON file reference in the codebase
rg "machine_JH_UniScheduler.json"
Length of output: 123
Script:
#!/bin/bash
# Description: List the contents of the "jsons" directory to check for the presence of the JSON file or similarly named files.
# List the contents of the "jsons" directory
if [ -d "jsons" ]; then
ls -l jsons
else
echo "Directory 'jsons' does not exist."
fi
Length of output: 101
There was a problem hiding this comment.
@njzjz, it appears that the jsons directory does not exist, which means the machine_JH_UniScheduler.json file is also missing. This would prevent the tests/devel_test_JH_UniScheduler.py script from running successfully. Please ensure that the directory and the required JSON file are present in the repository. If you need further assistance, feel free to ask!
Analysis chain
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the 'jsons' directory and the 'machine_JH_UniScheduler.json' file exist
if [ -d "jsons" ]; then
echo "Directory 'jsons' exists."
if [ -f "jsons/machine_JH_UniScheduler.json" ]; then
echo "File 'machine_JH_UniScheduler.json' exists."
else
echo "File 'machine_JH_UniScheduler.json' does not exist."
fi
else
echo "Directory 'jsons' does not exist."
fi
# Check for any logs or outputs indicating the test script has been executed
rg "Running tests/devel_test_JH_UniScheduler.py" --files-with-matches
Length of output: 173
tests/context.py
Outdated
| @@ -23,6 +23,7 @@ | |||
| from dpdispatcher.machines.pbs import PBS # noqa: F401 | |||
| from dpdispatcher.machines.shell import Shell # noqa: F401 | |||
| from dpdispatcher.machines.slurm import Slurm # noqa: F401 | |||
| from dpdispatcher.machines.JH_UniScheduler import JH_UniScheduler # noqa: F401 | |||
There was a problem hiding this comment.
Tip
Codebase Verification
The import statement for JH_UniScheduler is present in tests/context.py, but it is not utilized anywhere else in the file.
- Ensure that the new import
JH_UniScheduleris utilized appropriately in the context of this file.
Analysis chain
Ensure that the new import JH_UniScheduler is utilized appropriately in the context of this file.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the usage of JH_UniScheduler in the context file.
# Test: Search for the usage of JH_UniScheduler in the file.
rg --type python "JH_UniScheduler" tests/context.py
Length of output: 82
Script:
#!/bin/bash
# Description: Verify the usage of JH_UniScheduler in the context file.
# Test: Search for the usage of JH_UniScheduler in the file.
rg "JH_UniScheduler" tests/context.py
Length of output: 117
doc/index.rst
Outdated
There was a problem hiding this comment.
Please also add documentation to doc/batch.md.
tests/devel_test_JH_UniScheduler.py
Outdated
| with open("jsons/machine_JH_UniScheduler.json") as f: | ||
| mdata = json.load(f) | ||
|
|
| self.maxDiff = None | ||
|
|
||
| def test_shell_trival(self): | ||
| with open("jsons/machine_lazy_local_JH_UniScheduler.json") as f: |
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (3)
doc/batch.md (3)
Line range hint
80-80: Add missing article "the".- Read Fujitsu cloud service documentation for details. + Read the Fujitsu cloud service documentation for details.This change adds the missing article "the" to make the sentence grammatically correct.
Line range hint
85-85: Correct the verb form for clarity.- It using [AccessKey](https://bohrium.dp.tech/personal/setting) + It is using [AccessKey](https://bohrium.dp.tech/personal/setting)This change corrects the verb form to "It is using" for grammatical accuracy and clarity.
Line range hint
91-91: Address multiple grammatical issues in one sentence.- The commands and flags of SGE share a lot similarity with PBS except when checking job status. Use this argument if one is submitting job to SGE based batch system. + The commands and flags of SGE share a lot of similarity with PBS except when checking job status. Use this argument if one is submitting jobs to an SGE-based batch system.This change corrects several grammatical issues:
- Adds "of" after "a lot".
- Changes "job" to "jobs" and adds "an" before "SGE-based" for grammatical correctness.
doc/batch.md
Outdated
|
|
||
| [JH UniScheduler](http://www.jhinno.com/m/custom_case_05.html) was developed by JHINNO company and uses "jsub" to submit tasks. | ||
| Its overall architecture is similar to that of IBM's LSF. However, there are still some differences between them. One needs to | ||
| make sure LSF has been setup in the remote server and the related environment is activated. |
There was a problem hiding this comment.
Correct the setup terminology.
- make sure LSF has been setup in the remote server
+ make sure LSF has been set up in the remote serverThis change corrects the grammatical error from "setup" (noun) to "set up" (verb phrase), which is the intended usage here.
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.
| make sure LSF has been setup in the remote server and the related environment is activated. | |
| make sure LSF has been set up in the remote server and the related environment is activated. |
for more information, see https://pre-commit.ci
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #459 +/- ##
===========================================
- Coverage 60.59% 47.55% -13.05%
===========================================
Files 38 39 +1
Lines 3738 3821 +83
===========================================
- Hits 2265 1817 -448
- Misses 1473 2004 +531 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range and nitpick comments (5)
doc/batch.md (5)
Line range hint
21-21: Correct the grammatical error in the setup terminology.- make sure slurm has been setup in the remote server + make sure slurm has been set up in the remote server
Line range hint
33-33: Correct the grammatical error in the setup terminology.- make sure OpenPBS has been setup in the remote server + make sure OpenPBS has been set up in the remote server
Line range hint
43-43: Correct the grammatical error in the setup terminology.- make sure TORQUE has been setup in the remote server + make sure TORQUE has been set up in the remote server
Line range hint
85-85: Correct the grammatical structure for clarity.- It using [AccessKey](https://bohrium.dp.tech/personal/setting) + It is using [AccessKey](https://bohrium.dp.tech/personal/setting)
Line range hint
91-91: Correct the grammatical and syntactical errors.- The commands and flags of SGE share a lot similarity with PBS except when checking job status. Use this argument if one is submitting job to SGE based batch system. + The commands and flags of SGE share a lot of similarity with PBS, except when checking job status. Use this argument if one is submitting a job to an SGE-based batch system.
doc/batch.md
Outdated
|
|
||
| [JH UniScheduler](http://www.jhinno.com/m/custom_case_05.html) was developed by China's JHINNO company and uses "jsub" to submit tasks. | ||
| Its overall architecture is similar to that of IBM's LSF. However, there are still some differences between them. One needs to | ||
| make sure JH UniScheduler has been setup in the remote server and the related environment is activated. |
There was a problem hiding this comment.
Correct the grammatical error in the setup terminology.
- make sure JH UniScheduler has been setup in the remote server
+ make sure JH UniScheduler has been set up in the remote serverCommittable 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.
| make sure JH UniScheduler has been setup in the remote server and the related environment is activated. | |
| make sure JH UniScheduler has been set up in the remote server and the related environment is activated. |
doc/batch.md
Outdated
| @@ -49,6 +49,14 @@ | |||
| [IBM Spectrum LSF Suites](https://www.ibm.com/products/hpc-workload-management) is a comprehensive workload management solution used by HPCs. | |||
| One needs to make sure LSF has been setup in the remote server and the related environment is activated. | |||
There was a problem hiding this comment.
Correct the grammatical error in the setup terminology.
- make sure LSF has been setup in the remote server
+ make sure LSF has been set up in the remote serverCommittable 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.
| One needs to make sure LSF has been setup in the remote server and the related environment is activated. | |
| One needs to make sure LSF has been set up in the remote server and the related environment is activated. |
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (3)
doc/batch.md (3)
Line range hint
80-80: Add the missing article "the".- Read Fujitsu cloud service documentation for details. + Read the Fujitsu cloud service documentation for details.
Line range hint
85-85: Correct the auxiliary verb and article usage.- It using [AccessKey](https://bohrium.dp.tech/personal/setting) instead of username and password. + It is using [AccessKey](https://bohrium.dp.tech/personal/setting) instead of a username and password.
Line range hint
91-91: Correct grammatical errors and usage.- The commands and flags of SGE share a lot similarity with PBS except when checking job status. + The commands and flags of SGE share a lot of similarity with PBS except when checking job status. - Use this argument if one is submitting job to SGE based batch system. + Use this argument if one is submitting a job to an SGE-based batch system.
|
|
||
| [JH UniScheduler](http://www.jhinno.com/m/custom_case_05.html) was developed by China's JHINNO company and uses "jsub" to submit tasks. | ||
| Its overall architecture is similar to that of IBM's LSF. However, there are still some differences between them. One needs to | ||
| make sure JH UniScheduler has been set up in the remote server and the related environment is activated. |
There was a problem hiding this comment.
Correct the grammatical error in the setup terminology.
- make sure JH UniScheduler has been setup in the remote server
+ make sure JH UniScheduler has been set up in the remote serverCommittable 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.
| make sure JH UniScheduler has been set up in the remote server and the related environment is activated. | |
| make sure JH UniScheduler has been set up in the remote server and the related environment is activated. |
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (2)
tests/test_JH_UniScheduler_script_generation.py (2)
26-26: Consider renamingtest_shell_trivaltotest_shell_trivialto correct the typo in the method name.
26-137: Add explanatory comments to each major block of the test to improve readability and maintainability.
| task_list=task_list, | ||
| ) | ||
| submission.generate_jobs() | ||
| str = machine.gen_script_header(submission.belonging_jobs[0]) |
There was a problem hiding this comment.
Avoid using str as a variable name since it shadows a built-in Python type. Consider renaming it to something more descriptive like generated_header.
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (2)
doc/batch.md (2)
Line range hint
85-85: Correct the grammatical error in the description of OpenAPI.- It using [AccessKey](https://bohrium.dp.tech/personal/setting) + It is using [AccessKey](https://bohrium.dp.tech/personal/setting)
Line range hint
91-91: Correct the grammatical and syntactical errors in the description of SGE.- The commands and flags of SGE share a lot similarity with PBS except when checking job status. + The commands and flags of SGE share a lot of similarity with PBS except when checking job status. - Use this argument if one is submitting job to SGE based batch system. + Use this argument if one is submitting a job to an SGE-based batch system.
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (1)
doc/batch.md (1)
Line range hint
25-25: Add a comma for clarity.- Millions of Slurm jobs can be submitted quickly and Slurm can execute all Slurm jobs at the same time. + Millions of Slurm jobs can be submitted quickly, and Slurm can execute all Slurm jobs at the same time.
|
|
||
| ## SGE | ||
|
|
||
| {dargs:argument}`batch_type <resources/batch_type>`: `SGE` | ||
|
|
||
| The [Sun Grid Engine (SGE) scheduler](https://gridscheduler.sourceforge.net) is a batch-queueing system distributed resource management. The commands and flags of SGE share a lot similarity with PBS except when checking job status. Use this argument if one is submitting job to SGE based batch system. | ||
| The [Sun Grid Engine (SGE) scheduler](https://gridscheduler.sourceforge.net) is a batch-queueing system distributed resource management. The commands and flags of SGE share a lot of similarity with PBS except when checking job status. Use this argument if one is submitting job to an SGE-based batch system. |
There was a problem hiding this comment.
Add missing prepositions, commas, and articles for grammatical correctness.
- The Sun Grid Engine (SGE) scheduler is a batch-queueing system distributed resource management.
+ The Sun Grid Engine (SGE) scheduler is a batch-queueing system, distributed as resource management.
- The commands and flags of SGE share a lot of similarity with PBS except when checking job status.
+ The commands and flags of SGE share a lot of similarity with PBS, except when checking job status.
- Use this argument if one is submitting job to an SGE-based batch system.
+ Use this argument if one is submitting a job to an SGE-based batch system.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.
| The [Sun Grid Engine (SGE) scheduler](https://gridscheduler.sourceforge.net) is a batch-queueing system distributed resource management. The commands and flags of SGE share a lot of similarity with PBS except when checking job status. Use this argument if one is submitting job to an SGE-based batch system. | |
| The [Sun Grid Engine (SGE) scheduler](https://gridscheduler.sourceforge.net) is a batch-queueing system, distributed as resource management. The commands and flags of SGE share a lot of similarity with PBS, except when checking job status. Use this argument if one is submitting a job to an SGE-based batch system. |
Thank you for your comment. It already says "All checks have passed". Can we merge this branch and close this issue? Or what should be done next? |
JH UniScheduler was developed by JHINNO company. JH UniScheduler is a commercial software and uses "jsub" to submit tasks. Its overall architecture is similar to that of IBM's LSF. However, there are still some differences between them.
Summary by CodeRabbit
Release Notes
New Features
JH_UniScheduler, in the DPDispatcher Python package.JH_UniScheduler.Documentation
JH_UniScheduler.JH_UniScheduler.Testing
JH_UniScheduler.JH_UniScheduler.Configuration
jh_unischeduleras a keyword.