Use mpi-extensions main Open MPI in CI#834
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #834 +/- ##
=======================================
Coverage 90.63% 90.63%
=======================================
Files 15 15
Lines 491 491
Branches 183 183
=======================================
Hits 445 445
Misses 8 8
Partials 38 38 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@aobolensk please review |
allnes
changed the title
aobolensk
left a comment
aobolensk
left a comment
There was a problem hiding this comment.
General comment: what problem is being solved here, that is not solved by Docker image?
|
|
||
|
|
||
| def purge_linux_mpi() -> None: | ||
| if not shutil.which("apt-get"): |
There was a problem hiding this comment.
How do we deal with non-apt Linux distros?
| venv_dir = runner_temp / "mpi-extensions-python" | ||
| python_bin = venv_dir / "bin" / "python" | ||
| try: | ||
| run([sys.executable, "-m", "venv", str(venv_dir)]) |
There was a problem hiding this comment.
I don't like such commands hardcoding in this script. Why do we take control over the environment here? Can't we delegate some commands to the user. Pure black box is not a good idea
| @@ -0,0 +1,183 @@ | |||
| #!/usr/bin/env python3 | |||
|
|
|||
| from __future__ import annotations | |||
There was a problem hiding this comment.
Annotations are built in in Python 3, no need for that
| return | ||
| sudo = sudo_prefix() | ||
| if sudo is None: | ||
| print("::warning::sudo is unavailable; skipping Linux MPI package removal") |
There was a problem hiding this comment.
I'm really no sure we need to take require sudo for some python script. That is phishy
|
|
||
| apt_get = sudo + ["apt-get"] | ||
| run(apt_get + ["update"]) | ||
| run(apt_get + ["purge", "-y"] + MPI_PACKAGES_LINUX, check=False) |
There was a problem hiding this comment.
That's not good to enforce on user system.
2 participants
What changed
learning-process/mpi-extensionsmainprerelease in Linux/macOS CI instead of package-manager MPI.scripts/install_mpi_extensions.pyand a reusable setup action wrapper to download, verify, and unpack the selected release asset.-D PPC_MPI_EXTENSIONS_HOME=/path/to/mpi-extensions-openmpifor Linux/macOS CMake builds when C++ components are enabled.ppc_mpi_runtime_env.json;scripts/run_tests.pyconsumes that file and sets the required runtime environment itself.User impact
A developer or CI job can unpack the mpi-extensions Open MPI archive anywhere and pass only:
cmake -S . -B build -D PPC_MPI_EXTENSIONS_HOME=/path/to/mpi-extensions-openmpiNo Homebrew/apt MPI install and no manual
PATH,LD_LIBRARY_PATH,DYLD_LIBRARY_PATH,MPI_HOME, orOPAL_PREFIXexports are required for Linux/macOS course builds/tests.Validation
mainOpen MPI package, verified CMake fails withoutPPC_MPI_EXTENSIONS_HOME, configured/built/installedPPC_TASKS=examplewith ccache, and ran MPI tests viascripts/run_tests.pywithout MPI exports.ubuntu-x86): installed the current mpi-extensionsmainOpen MPI package, verified CMake fails withoutPPC_MPI_EXTENSIONS_HOME, configured/built/installedPPC_TASKS=examplewith ccache, and ran MPI tests viascripts/run_tests.pywithout MPI exports.isort,ruff,ruff format --check,flake8,cmake-format,yamllint,check-yaml,editorconfig-checker,py_compile,git diff --check, andactionlintfor workflows.Notes
pre-commit runas a single full command is blocked locally by this Mac's Node/npm engine mismatch in the markdownlint hook. The affected Python/CMake/YAML/editorconfig hooks were run directly.