Skip to content
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.

Sign up for GitHub

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

Spack CPU build exago~mpi #47

Draft
wants to merge 6 commits into
base: develop
Choose a base branch
from
Draft

Spack CPU build exago~mpi #47

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
08a713f
Add back Spack CPU builds to reproduce issues
cameronrutherford Oct 13, 2023
a014957
Use container's spack and load spack binary cache
cameronrutherford Oct 16, 2023
aec2713
Only check for Python if Python is enabled
cameronrutherford Oct 16, 2023
64c9d64
Properly check for tests/MPI in including Python wrapper extensions
cameronrutherford Oct 16, 2023
51d6802
Apply pre-commmit fixes
cameronrutherford Oct 16, 2023
6dcc180
Update exago_python.hpp to conditionally include mpi4py
cameronrutherford Oct 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 8 additions & 9 deletions .github/workflows/spack_cpu_build.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,14 @@ jobs:
# Minimal Build(s)
# Need S3 mirror to have these builds speedup
spack_spec:
# See #39 - ~python~mpi causes issues
# - exago@develop~mpi~ipopt~hiop~python~raja
# See #44 - +mpi~python should fail if no python in system,
# but the runner happens to have one...
- exago@develop+mpi~ipopt+hiop~python~raja ^openmpi
# See #39 - minimal build useful for sanity
- exago@develop~mpi~ipopt~hiop~python~raja
# See #16 / #44 - +python~mpi catches mpi4py / python config
- exago@develop~mpi~ipopt~hiop+python~raja
Comment on lines +17 to +20
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue in +mpi~python should be equivalent to a build with ~mpi~python as far as I can tell. Either way I think this is sufficient and we need to ensure these builds pass in future.

# See #18 - +hiop~mpi causes issues
# - exago@develop~mpi~ipopt+hiop~python+raja
# See #16 - +python~mpi causes issues
# - exago@develop~mpi~ipopt+hiop+python~raja
- exago@develop+mpi~ipopt+hiop+python~raja ^openmpi
# See #40 - +hiop+raja~ipopt ^hiop~sparse is useful for edge cases
# See #40 - +hiop+raja~ipopt ^hiop~sparse is useful for edge cases in xSDK
- exago@develop+mpi~ipopt+hiop~python+raja ^openmpi ^hiop+raja~sparse

name: Build ExaGO with Spack
Expand All @@ -41,8 +38,10 @@ jobs:
SPACK_SPEC: ${{ matrix.spack_spec }}
run: |
ls && pwd
. ./tpl/spack/share/spack/setup-env.sh
. /opt/spack/share/spack/setup-env.sh
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is more sustainable, and can be swapped out when we want to use ExaGO's spack.

It might just make more sense to have a package.py stored in the repo for ExaGO as that is the main thing that is regularly modified instead of relying on submodules. Still would be nice to pin a spack version and support that workflow, but making it an optional clone would be best.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The spack repo is is incredibly active, I would be hesitant to pin a version of spack because that's just another package we would could get hung up on a "blessed" version or forget to upgrade that one clone...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To clarify, for 100% reproducible spack builds we need to pin the version in certain situations (such as profiling runs, or key performance runs).

For CI and automated builds, I do think that just using spack's develop like we do with this base image is the best way to go.

Is that in line with what you were thinking?

spack debug report
spack mirror add spack_public_mirror https://binaries.spack.io/develop
spack buildcache keys --install --trust
spack env create -d ./spack-env
spack env activate ./spack-env
spack add $SPACK_SPEC target=x86_64
Expand Down
2 changes: 1 addition & 1 deletion CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -406,8 +406,8 @@ else()
endif()
endif()

include(ExaGOCheckPython)
if(EXAGO_ENABLE_PYTHON)
include(ExaGOCheckPython)
# Build pybind11 target manually
add_subdirectory(tpl/pybind11)
add_subdirectory(interfaces/python)
Expand Down
97 changes: 49 additions & 48 deletions interfaces/python/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,33 +31,19 @@ target_link_libraries(exago PRIVATE ${EXAGO_PY_LIBS})
# Find mpi4py c headers
include(ExaGOFindPythonModule)

find_python_module(mpi4py REQUIRED)

# Get the include directories needed
if(mpi4py_FOUND)
execute_process(
COMMAND "${Python_EXECUTABLE}" "-c"
"import mpi4py as m; print(m.__version__); print(m.get_include());"
RESULT_VARIABLE _mpi4py_SEARCH_SUCCESS
OUTPUT_VARIABLE _mpi4py_VALUES
ERROR_VARIABLE _mpi4py_ERROR_VALUE
OUTPUT_STRIP_TRAILING_WHITESPACE
)

# Find pytest c headers
find_python_module(pytest REQUIRED)

if(EXAGO_ENABLE_MPI)
find_python_module(mpi4py REQUIRED)
# Get the include directories needed
if(pytest_FOUND)
if(mpi4py_FOUND)
execute_process(
COMMAND "${Python_EXECUTABLE}" "-c"
"import pytest as p; print(p.__version__);"
RESULT_VARIABLE _pytest_SEARCH_SUCCESS
OUTPUT_VARIABLE _pytest_VALUES
ERROR_VARIABLE _pytest_ERROR_VALUE
COMMAND
"${Python_EXECUTABLE}" "-c"
"import mpi4py as m; print(m.__version__); print(m.get_include());"
RESULT_VARIABLE _mpi4py_SEARCH_SUCCESS
OUTPUT_VARIABLE _mpi4py_VALUES
ERROR_VARIABLE _mpi4py_ERROR_VALUE
OUTPUT_STRIP_TRAILING_WHITESPACE
)

# Convert the process output into a list
string(REGEX REPLACE ";" "\\\\;" _mpi4py_VALUES ${_mpi4py_VALUES})
string(REGEX REPLACE "\n" ";" _mpi4py_VALUES ${_mpi4py_VALUES})
Expand All @@ -78,32 +64,47 @@ if(mpi4py_FOUND)
mpi4py_VERSION_DECIMAL
"(${mpi4py_VERSION_MAJOR} * 10000) + (${mpi4py_VERSION_MINOR} * 100) + ${mpi4py_VERSION_PATCH}"
)
endif()

target_include_directories(exago SYSTEM PRIVATE ${mpi4py_INCLUDE_DIRS})

string(REGEX REPLACE ";" "\\\\;" _pytest_VALUES ${_pytest_VALUES})
string(REGEX REPLACE "\n" ";" _pytest_VALUES ${_pytest_VALUES})
list(GET _pytest_VALUES 0 pytest_VERSION)
list(GET _pytest_VALUES 0 pytest_INCLUDE_DIRS)

# Make sure all directory separators are '/'
string(REGEX REPLACE "\\\\" "/" pytest_INCLUDE_DIRS ${pytest_INCLUDE_DIRS})

# Get the major and minor version numbers
string(REGEX REPLACE "\\." ";" _pytest_VERSION_LIST ${pytest_VERSION})
list(GET _pytest_VERSION_LIST 0 pytest_VERSION_MAJOR)
list(GET _pytest_VERSION_LIST 1 pytest_VERSION_MINOR)
list(GET _pytest_VERSION_LIST 2 pytest_VERSION_PATCH)
string(REGEX MATCH "[0-9]*" pytest_VERSION_PATCH ${pytest_VERSION_PATCH})
math(
EXPR
pytest_VERSION_DECIMAL
"(${pytest_VERSION_MAJOR} * 10000) + (${pytest_VERSION_MINOR} * 100) + ${pytest_VERSION_PATCH}"
)
endif()

target_include_directories(exago SYSTEM PRIVATE ${pytest_INCLUDE_DIRS})
target_include_directories(exago SYSTEM PRIVATE ${mpi4py_INCLUDE_DIRS})
endif(mpi4py_FOUND)
endif(EXAGO_ENABLE_MPI)

if(EXAGO_RUN_TESTS)
# Find pytest c headers
find_python_module(pytest REQUIRED)

# Get the include directories needed
if(pytest_FOUND)
execute_process(
COMMAND "${Python_EXECUTABLE}" "-c"
"import pytest as p; print(p.__version__);"
RESULT_VARIABLE _pytest_SEARCH_SUCCESS
OUTPUT_VARIABLE _pytest_VALUES
ERROR_VARIABLE _pytest_ERROR_VALUE
OUTPUT_STRIP_TRAILING_WHITESPACE
)
string(REGEX REPLACE ";" "\\\\;" _pytest_VALUES ${_pytest_VALUES})
string(REGEX REPLACE "\n" ";" _pytest_VALUES ${_pytest_VALUES})
list(GET _pytest_VALUES 0 pytest_VERSION)
list(GET _pytest_VALUES 0 pytest_INCLUDE_DIRS)

# Make sure all directory separators are '/'
string(REGEX REPLACE "\\\\" "/" pytest_INCLUDE_DIRS ${pytest_INCLUDE_DIRS})

# Get the major and minor version numbers
string(REGEX REPLACE "\\." ";" _pytest_VERSION_LIST ${pytest_VERSION})
list(GET _pytest_VERSION_LIST 0 pytest_VERSION_MAJOR)
list(GET _pytest_VERSION_LIST 1 pytest_VERSION_MINOR)
list(GET _pytest_VERSION_LIST 2 pytest_VERSION_PATCH)
string(REGEX MATCH "[0-9]*" pytest_VERSION_PATCH ${pytest_VERSION_PATCH})
math(
EXPR
pytest_VERSION_DECIMAL
"(${pytest_VERSION_MAJOR} * 10000) + (${pytest_VERSION_MINOR} * 100) + ${pytest_VERSION_PATCH}"
)
target_include_directories(exago SYSTEM PRIVATE ${pytest_INCLUDE_DIRS})
endif(pytest_FOUND)
endif(EXAGO_RUN_TESTS)

# Install Python bindings to same installation path:
install(TARGETS exago DESTINATION ${EXAGO_PYTHON_SITELIB})
3 changes: 3 additions & 0 deletions interfaces/python/exago_python.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,11 @@
#include <pybind11/stl.h>
#include <pybind11/stl_bind.h>

#ifdef EXAGO_ENABLE_MPI
#include <mpi.h>
#include <mpi4py/mpi4py.h>
#endif

#include <utils.h>
#include <string.h>

Expand Down
Loading