Release openmpi constraint - Test with openmpi 5.0 #337
Conversation
WalkthroughThe updates primarily focus on enhancing flexibility and integration within the project. The OpenMPI dependency version is relaxed across several configuration files. New GitHub Actions workflows are introduced for unit testing with Flux-MPICH and Flux-OpenMPI. The installation documentation and core code are updated to support the Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![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
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- .ci_support/environment-openmpi.yml (1 hunks)
- binder/environment.yml (1 hunks)
Files skipped from review due to trivial changes (2)
- .ci_support/environment-openmpi.yml
- binder/environment.yml
|
|
Waiting for conda-forge/staged-recipes#26478 |
|
I continue to debug this by testing from mpi4py import MPI
comm = MPI.COMM_WORLD
size = comm.Get_size()
rank = comm.Get_rank()
print(comm, size, rank)When I run this with But when I run it with |
for more information, see https://pre-commit.ci
…openmpi_constraint # Conflicts: # pympipool/shared/inputcheck.py
for more information, see https://pre-commit.ci
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
… release_openmpi_constraint
…openmpi_constraint # Conflicts: # pympipool/scheduler/__init__.py # pympipool/shared/inputcheck.py # tests/test_shared_input_check.py
There was a problem hiding this comment.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (13)
- .github/workflows/benchmark.yml (1 hunks)
- .github/workflows/unittest-flux-mpich.yml (1 hunks)
- .github/workflows/unittest-flux-openmpi.yml (1 hunks)
- binder/environment.yml (1 hunks)
- docs/installation.md (1 hunks)
- notebooks/examples.ipynb (1 hunks)
- pympipool/init.py (6 hunks)
- pympipool/scheduler/init.py (5 hunks)
- pympipool/scheduler/flux.py (9 hunks)
- pympipool/shared/inputcheck.py (2 hunks)
- tests/test_executor_backend_flux.py (3 hunks)
- tests/test_flux_executor.py (2 hunks)
- tests/test_shared_input_check.py (1 hunks)
Files not summarized due to errors (1)
- notebooks/examples.ipynb: Error: Message exceeds token limit
Files skipped from review due to trivial changes (2)
- .github/workflows/benchmark.yml
- tests/test_shared_input_check.py
Additional Context Used
LanguageTool (17)
docs/installation.md (17)
Near line 9: A comma might be missing here.
Context: ...RM allocation using thesruncommand. Still this always queries the central databa...
Rule ID: AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA
Near line 10: A comma might be missing here.
Context: ...s the central database of the SLURM job scheduler which can decrease the performance of t...
Rule ID: AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA
Near line 16: A comma might be missing here.
Context: ...for GPUs from your vendor. In the same way the version ofpmifor your queuing s...
Rule ID: AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA
Near line 21: A comma might be missing here.
Context: ... ### AMD GPUs with mpich / cray mpi For example the [Frontier HPC](https://www.olcf.orn...
Rule ID: AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA
Near line 22: The preposition “with” seems more likely in this position.
Context: ...th cray mpi version which is compatible to mpich4.X. So the corresponding versi...
Rule ID: AI_EN_LECTOR_REPLACEMENT_PREPOSITION
Near line 28: A comma might be missing here.
Context: ... Nvidia GPUs with mpich / cray mpi For example the [Perlmutter HPC](https://docs.nersc...
Rule ID: AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA
Near line 29: The preposition “with” seems more likely in this position.
Context: ...ation with cray mpi which is compatible to mpich4.X. So the corresponding vers...
Rule ID: AI_EN_LECTOR_REPLACEMENT_PREPOSITION
Near line 34: A comma might be missing here.
Context: ...en installing on a login node without a GPU the conda install command might fail wi...
Rule ID: AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA
Near line 43: A comma might be missing here.
Context: ...# Intel GPUs with mpich / cray mpi For example the [Aurora HPC](https://www.alcf.anl.g...
Rule ID: AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA
Near line 44: The preposition “with” seems more likely in this position.
Context: ...ation with cray mpi which is compatible to mpich4.X. So the corresponding versi...
Rule ID: AI_EN_LECTOR_REPLACEMENT_PREPOSITION
Near line 51: The preposition ‘for’ seems more likely in this position.
Context: ...ch / cray mpi, it can also be installed in compatibility with openmpi or intel mpi...
Rule ID: AI_HYDRA_LEO_REPLACE_IN_FOR
Near line 68: It appears that a comma is missing.
Context: ...the flux scheduler. In this interactive shell you can now list the available resourc...
Rule ID: DURING_THAT_TIME_COMMA
Near line 85: Possible missing comma found.
Context: ...e resources to the flux framework. For SLURM this is achieved by calling `flux start...
Rule ID: AI_HYDRA_LEO_MISSING_COMMA
Near line 104: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...). While this is not recommended in the high performance computing (HPC) context aspympipool...
Rule ID: EN_COMPOUND_ADJECTIVE_INTERNAL
Near line 104: Possible missing comma found.
Context: ...in the high performance computing (HPC) context aspympipoolwith `block_allocation=F...
Rule ID: AI_HYDRA_LEO_MISSING_COMMA
Near line 106: Consider inserting a comma after an introductory phrase for better readability.
Context: ...p for each submitted python function. In this casepympipoolcan be installed using: ```...
Rule ID: IN_THAT_CASE_COMMA
Near line 112: The operating system from Apple is written “macOS”.
Context: ...orkstation installations on Windows and MacOS.
Rule ID: MAC_OS
Ruff (2)
tests/test_executor_backend_flux.py (2)
12-12:
pympipool.scheduler.flux.PyFluxExecutorimported but unused; consider usingimportlib.util.find_specto test for availability
13-13:
pympipool.scheduler.flux.FluxPythonInterfaceimported but unused; consider usingimportlib.util.find_specto test for availability
Markdownlint (83)
docs/installation.md (83)
3: Expected: 0 or 2; Actual: 1
Trailing spaces
4: Expected: 0 or 2; Actual: 1
Trailing spaces
7: Expected: 0 or 2; Actual: 1
Trailing spaces
9: Expected: 0 or 2; Actual: 1
Trailing spaces
10: Expected: 0 or 2; Actual: 1
Trailing spaces
15: Expected: 0 or 2; Actual: 1
Trailing spaces
16: Expected: 0 or 2; Actual: 1
Trailing spaces
17: Expected: 0 or 2; Actual: 1
Trailing spaces
18: Expected: 0 or 2; Actual: 1
Trailing spaces
21: Expected: 0 or 2; Actual: 1
Trailing spaces
23: Expected: 0 or 2; Actual: 1
Trailing spaces
27: Expected: 0 or 2; Actual: 1
Trailing spaces
28: Expected: 0 or 2; Actual: 1
Trailing spaces
29: Expected: 0 or 2; Actual: 1
Trailing spaces
30: Expected: 0 or 2; Actual: 1
Trailing spaces
39: Expected: 0 or 2; Actual: 1
Trailing spaces
40: Expected: 0 or 2; Actual: 1
Trailing spaces
42: Expected: 0 or 2; Actual: 1
Trailing spaces
43: Expected: 0 or 2; Actual: 1
Trailing spaces
44: Expected: 0 or 2; Actual: 1
Trailing spaces
45: Expected: 0 or 2; Actual: 1
Trailing spaces
51: Expected: 0 or 2; Actual: 1
Trailing spaces
52: Expected: 0 or 2; Actual: 1
Trailing spaces
63: Expected: 0 or 2; Actual: 1
Trailing spaces
68: Expected: 0 or 2; Actual: 1
Trailing spaces
69: Expected: 0 or 2; Actual: 1
Trailing spaces
80: Expected: 0 or 2; Actual: 1
Trailing spaces
81: Expected: 0 or 2; Actual: 1
Trailing spaces
84: Expected: 0 or 2; Actual: 1
Trailing spaces
89: Expected: 0 or 2; Actual: 1
Trailing spaces
94: Expected: 0 or 2; Actual: 1
Trailing spaces
95: Expected: 0 or 2; Actual: 1
Trailing spaces
96: Expected: 0 or 2; Actual: 1
Trailing spaces
105: Expected: 0 or 2; Actual: 1
Trailing spaces
1: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines
2: Expected: 1; Actual: 0; Above
Headings should be surrounded by blank lines
2: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines
13: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines
20: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines
27: Expected: 1; Actual: 0; Above
Headings should be surrounded by blank lines
27: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines
42: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines
50: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines
62: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines
83: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines
94: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines
101: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines
24: null
Fenced code blocks should be surrounded by blank lines
26: null
Fenced code blocks should be surrounded by blank lines
31: null
Fenced code blocks should be surrounded by blank lines
33: null
Fenced code blocks should be surrounded by blank lines
36: null
Fenced code blocks should be surrounded by blank lines
38: null
Fenced code blocks should be surrounded by blank lines
46: null
Fenced code blocks should be surrounded by blank lines
53: null
Fenced code blocks should be surrounded by blank lines
55: null
Fenced code blocks should be surrounded by blank lines
57: null
Fenced code blocks should be surrounded by blank lines
59: null
Fenced code blocks should be surrounded by blank lines
65: null
Fenced code blocks should be surrounded by blank lines
67: null
Fenced code blocks should be surrounded by blank lines
70: null
Fenced code blocks should be surrounded by blank lines
72: null
Fenced code blocks should be surrounded by blank lines
74: null
Fenced code blocks should be surrounded by blank lines
79: null
Fenced code blocks should be surrounded by blank lines
86: null
Fenced code blocks should be surrounded by blank lines
88: null
Fenced code blocks should be surrounded by blank lines
90: null
Fenced code blocks should be surrounded by blank lines
97: null
Fenced code blocks should be surrounded by blank lines
108: null
Fenced code blocks should be surrounded by blank lines
9: null
Spaces inside code span elements
24: null
Fenced code blocks should have a language specified
31: null
Fenced code blocks should have a language specified
36: null
Fenced code blocks should have a language specified
46: null
Fenced code blocks should have a language specified
53: null
Fenced code blocks should have a language specified
57: null
Fenced code blocks should have a language specified
65: null
Fenced code blocks should have a language specified
70: null
Fenced code blocks should have a language specified
74: null
Fenced code blocks should have a language specified
86: null
Fenced code blocks should have a language specified
90: null
Fenced code blocks should have a language specified
97: null
Fenced code blocks should have a language specified
108: null
Fenced code blocks should have a language specified
Additional comments not posted (14)
binder/environment.yml (2)
6-6: The removal of the version constraint onopenmpialigns with the PR's objectives and should facilitate broader compatibility testing.
12-12: Addingflux-pmixas a dependency is appropriate for ensuring compatibility with OpenMPI version 5.0..github/workflows/unittest-flux-mpich.yml (1)
18-19: Properly extending the environment to include necessary dependencies likeflux-coreandversioneerensures that the testing environment is correctly set up..github/workflows/unittest-flux-openmpi.yml (1)
18-19: Includingflux-pmixin the environment setup is essential for compatibility testing with OpenMPI version 5.0, ensuring the testing environment is correctly configured.tests/test_executor_backend_flux.py (1)
85-85: The addition of thepmiparameter to the test functions is necessary for testing the new PMI interface functionality in thepympipoolpackage.Also applies to: 98-98
pympipool/shared/inputcheck.py (1)
88-94: The addition of thecheck_pmifunction is appropriate for validating thepmiparameter based on the backend, ensuring compatibility with OpenMPI v5.tests/test_flux_executor.py (1)
73-73: The addition of thepmiparameter to the test functions is necessary for testing the new PMI interface functionality in thepympipoolpackage.Also applies to: 81-81
docs/installation.md (2)
54-59: Update the installation instructions to reflect the new OpenMPI version and the requiredflux-pmixplugin.The updated instructions clearly guide users on how to install the necessary components for OpenMPI version 5, including the new
flux-pmixplugin. This is crucial for ensuring compatibility and proper functionality.
60-60: Clarify the usage of thepmi="pmix"parameter in thepympipool.Executor.Ensure that the
pmi="pmix"parameter is correctly implemented in thepympipool.Executorto handle the new backend requirements.pympipool/scheduler/__init__.py (2)
54-54: Ensure the newpmiparameter is correctly integrated across different executor types.The addition of the
pmiparameter in thecreate_executorfunction and its documentation is well-integrated. This change supports the new requirements for OpenMPI v5 and enhances flexibility in backend management.Also applies to: 88-88
Line range hint
96-123: Validate thepmiparameter handling in thePyFluxExecutorandPyFluxStepExecutor.This script checks that the
pmiparameter is being correctly passed and utilized within thePyFluxExecutorandPyFluxStepExecutorclasses, ensuring compatibility with OpenMPI v5.Verification successful
The
__init__methods for bothPyFluxExecutorandPyFluxStepExecutorclasses do include thepmiparameter. This confirms that thepmiparameter is being handled in the constructors of these classes.
PyFluxExecutorclass:
__init__method includespmi: Optional[str] = NonePyFluxStepExecutorclass:
__init__method includespmi: Optional[str] = NoneThis indicates that the
pmiparameter is indeed being passed and utilized within these classes.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correct handling of the `pmi` parameter in the `PyFluxExecutor` and `PyFluxStepExecutor`. # Test: Search for the usage of the `pmi` parameter in these classes. ast-grep --lang python --pattern $'class PyFluxExecutor { $$$ __init__($_, $_, pmi=$_) { $$$ } $$$ }'Length of output: 461
Script:
#!/bin/bash # Description: Locate the definitions of the `PyFluxExecutor` and `PyFluxStepExecutor` classes and verify the `pmi` parameter handling. # Step 1: Search for the definitions of `PyFluxExecutor` and `PyFluxStepExecutor` classes. echo "Searching for PyFluxExecutor class definition..." ast-grep --lang python --pattern 'class PyFluxExecutor { $$$ }' echo "Searching for PyFluxStepExecutor class definition..." ast-grep --lang python --pattern 'class PyFluxStepExecutor { $$$ }' # Step 2: Verify the `__init__` methods of these classes for the `pmi` parameter. echo "Verifying __init__ method of PyFluxExecutor for pmi parameter..." ast-grep --lang python --pattern 'class PyFluxExecutor { $$$ __init__($_, $_, pmi=$_) { $$$ } $$$ }' echo "Verifying __init__ method of PyFluxStepExecutor for pmi parameter..." ast-grep --lang python --pattern 'class PyFluxStepExecutor { $$$ __init__($_, $_, pmi=$_) { $$$ } $$$ }'Length of output: 2449
Script:
#!/bin/bash # Description: Locate the definitions of the `PyFluxExecutor` and `PyFluxStepExecutor` classes and verify the `pmi` parameter handling. # Step 1: Search for the definitions of `PyFluxExecutor` and `PyFluxStepExecutor` classes. echo "Searching for PyFluxExecutor class definition..." rg 'class PyFluxExecutor' echo "Searching for PyFluxStepExecutor class definition..." rg 'class PyFluxStepExecutor' # Step 2: Verify the `__init__` methods of these classes for the `pmi` parameter. echo "Verifying __init__ method of PyFluxExecutor for pmi parameter..." rg 'def __init__.*pmi' -A 10 echo "Verifying __init__ method of PyFluxStepExecutor for pmi parameter..." rg 'def __init__.*pmi' -A 10Length of output: 764
Script:
#!/bin/bash # Description: Locate the `__init__` methods of the `PyFluxExecutor` and `PyFluxStepExecutor` classes and verify their parameters. # Step 1: Locate the `__init__` methods of `PyFluxExecutor` and `PyFluxStepExecutor` classes. echo "Locating __init__ method of PyFluxExecutor..." rg 'def __init__' pympipool/scheduler/flux.py -A 10 echo "Locating __init__ method of PyFluxStepExecutor..." rg 'def __init__' pympipool/scheduler/flux.py -A 10Length of output: 2465
pympipool/scheduler/flux.py (2)
31-31: Consistent integration of thepmiparameter across Flux executor classes.The consistent addition of the
pmiparameter across various classes within theflux.pymodule supports the new OpenMPI v5 requirements. This ensures that the PMI interface can be appropriately configured, enhancing the module's flexibility and compatibility.Also applies to: 70-70, 112-112, 148-148, 182-182
Line range hint
192-212: Ensure thepmiparameter is correctly utilized in theFluxPythonInterface.This script checks that the
pmiparameter is being correctly utilized in theFluxPythonInterface, ensuring that it is properly set in the job specifications when provided.Verification successful
The
pmiparameter is correctly utilized in theFluxPythonInterfaceclass. The__init__method inpympipool/scheduler/flux.pyincludes thepmiparameter, and it is used in thesetattr_shell_optionmethod.
pympipool/scheduler/flux.py: Thepmiparameter is set in the job specification usingjobspec.setattr_shell_option("pmi", self._pmi).Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correct utilization of the `pmi` parameter in the `FluxPythonInterface`. # Test: Search for the implementation and usage of the `pmi` parameter in the `FluxPythonInterface`. ast-grep --lang python --pattern $'class FluxPythonInterface { $$$ __init__($_, $_, pmi=$_) { $$$ } $$$ }'Length of output: 466
Script:
#!/bin/bash # Locate the FluxPythonInterface class definition ast-grep --lang python --pattern 'class FluxPythonInterface { $$$ }' # Locate the __init__ method within the FluxPythonInterface class and check for the pmi parameter ast-grep --lang python --pattern 'class FluxPythonInterface { $$$ def __init__($_, $_, pmi=$_) { $$$ } $$$ }' # Check for the usage of the pmi parameter within the FluxPythonInterface class ast-grep --lang python --pattern 'class FluxPythonInterface { $$$ setattr_shell_option("pmi", $_) $$$ }'Length of output: 1435
Script:
#!/bin/bash # Locate the FluxPythonInterface class definition rg 'class FluxPythonInterface' --glob '*.py' # Locate the __init__ method within the FluxPythonInterface class and check for the pmi parameter rg 'def __init__' --glob '*.py' -A 5 # Check for the usage of the pmi parameter within the FluxPythonInterface class rg 'setattr_shell_option\("pmi"' --glob '*.py'Length of output: 7007
pympipool/__init__.py (1)
51-51: Integration of thepmiparameter in theExecutorclass to support OpenMPI v5.The addition of the
pmiparameter in theExecutorclass is well-documented and correctly implemented. This change is crucial for ensuring compatibility with OpenMPI v5, particularly when using thefluxbackend.Also applies to: 89-89, 111-111, 149-149, 169-169, 188-188
Summary by CodeRabbit
New Features
pmiparameter.Updates
flux-pmixdependency.pmiparameter settings.Bug Fixes
Tests
pmiparameter functionality.