Skip to content
This repository was archived by the owner on Mar 20, 2023. It is now read-only.

Commit f3d4615

Browse files
authored
Improve Python wheel support (#785)
- Use new `NRN_WHEEL_BUILD` option to avoid adding a build flag that is not widely supported. This option is added in neuronsimulator/nrn#1661. - Make `nrnivmodl -coreneuron` fail in GPU builds if a non-NVC++ compiler is used. - Make `nrnivmodl -coreneuron` fail in GPU builds if a different NVC++ version is used to the one used to build the wheel. - Move some pre-existing logic into a new `cnrn_parse_version` CMake function; use it to get `major.minor` version numbers from CUDA and NVHPC versions that have `.patch` parts too. - Modify `CORENEURON_LIB_LINK_FLAGS` to use `-L$(libdir)`, which catches an edge case when a patched `libnrniv.so` is being used so the relevant path was not already set. - Make the logic for translating CMake target dependencies into linker flags more verbose. - Don't print single-MPI paths in dynamic MPI mode. - Don't add `-I` for MPI headers in dynamic MPI builds.
1 parent 2d08705 commit f3d4615

File tree

3 files changed

+63
-19
lines changed

3 files changed

+63
-19
lines changed

CMake/OpenAccHelper.cmake

Lines changed: 27 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,35 @@
44
# See top-level LICENSE file for details.
55
# =============================================================================
66

7+
# Helper to parse X.Y[.{anything] into X.Y
8+
function(cnrn_parse_version FULL_VERSION)
9+
cmake_parse_arguments(PARSE_ARGV 1 CNRN_PARSE_VERSION "" "OUTPUT_MAJOR_MINOR" "")
10+
if(NOT "${CNRN_PARSE_VERSION_UNPARSED_ARGUMENTS}" STREQUAL "")
11+
message(
12+
FATAL_ERROR
13+
"cnrn_parse_version got unexpected arguments: ${CNRN_PARSE_VERSION_UNPARSED_ARGUMENTS}")
14+
endif()
15+
string(FIND ${FULL_VERSION} . first_dot)
16+
math(EXPR first_dot_plus_one "${first_dot}+1")
17+
string(SUBSTRING ${FULL_VERSION} ${first_dot_plus_one} -1 minor_and_later)
18+
string(FIND ${minor_and_later} . second_dot_relative)
19+
if(${first_dot} EQUAL -1 OR ${second_dot_relative} EQUAL -1)
20+
message(FATAL_ERROR "Failed to parse major.minor from ${FULL_VERSION}")
21+
endif()
22+
math(EXPR second_dot_plus_one "${first_dot}+${second_dot_relative}+1")
23+
string(SUBSTRING ${FULL_VERSION} 0 ${second_dot_plus_one} major_minor)
24+
set(${CNRN_PARSE_VERSION_OUTPUT_MAJOR_MINOR}
25+
${major_minor}
26+
PARENT_SCOPE)
27+
endfunction()
28+
729
# =============================================================================
830
# Prepare compiler flags for GPU target
931
# =============================================================================
1032
if(CORENRN_ENABLE_GPU)
33+
# Get the NVC++ version number for use in nrnivmodl_core_makefile.in
34+
cnrn_parse_version(${CMAKE_CXX_COMPILER_VERSION} OUTPUT_MAJOR_MINOR
35+
CORENRN_NVHPC_MAJOR_MINOR_VERSION)
1136
# Enable cudaProfiler{Start,Stop}() behind the Instrumentor::phase... APIs
1237
add_compile_definitions(CORENEURON_CUDA_PROFILING CORENEURON_ENABLE_GPU)
1338
# Plain C++ code in CoreNEURON may need to use CUDA runtime APIs for, for example, starting and
@@ -23,20 +48,7 @@ if(CORENRN_ENABLE_GPU)
2348
if(NOT ${CMAKE_CUDA_COMPILER_ID} STREQUAL "NVIDIA")
2449
message(FATAL_ERROR "Unsupported CUDA compiler ${CMAKE_CUDA_COMPILER_ID}")
2550
endif()
26-
# Parse CMAKE_CUDA_COMPILER_VERSION=x.y.z into CUDA_VERSION=x.y
27-
string(FIND ${CMAKE_CUDA_COMPILER_VERSION} . first_dot)
28-
math(EXPR first_dot_plus_one "${first_dot}+1")
29-
string(SUBSTRING ${CMAKE_CUDA_COMPILER_VERSION} ${first_dot_plus_one} -1 minor_and_later)
30-
string(FIND ${minor_and_later} . second_dot_relative)
31-
if(${first_dot} EQUAL -1 OR ${second_dot_relative} EQUAL -1)
32-
message(
33-
FATAL_ERROR
34-
"Failed to parse a CUDA_VERSION from CMAKE_CUDA_COMPILER_VERSION=${CMAKE_CUDA_COMPILER_VERSION}"
35-
)
36-
endif()
37-
math(EXPR second_dot_plus_one "${first_dot}+${second_dot_relative}+1")
38-
string(SUBSTRING ${CMAKE_CUDA_COMPILER_VERSION} 0 ${second_dot_plus_one}
39-
CORENRN_CUDA_VERSION_SHORT)
51+
cnrn_parse_version(${CMAKE_CUDA_COMPILER_VERSION} OUTPUT_MAJOR_MINOR CORENRN_CUDA_VERSION_SHORT)
4052
else()
4153
# This is a lazy way of getting the major/minor versions separately without parsing
4254
# ${CMAKE_CUDA_COMPILER_VERSION}
@@ -91,7 +103,7 @@ if(CORENRN_ENABLE_GPU)
91103
GLOBAL
92104
PROPERTY
93105
CORENEURON_LIB_LINK_FLAGS
94-
"${NVHPC_ACC_COMP_FLAGS} -rdynamic -lrt -Wl,--whole-archive -L${CMAKE_HOST_SYSTEM_PROCESSOR} -lcorenrnmech -L${CMAKE_INSTALL_PREFIX}/lib -lcoreneuron -Wl,--no-whole-archive"
106+
"${NVHPC_ACC_COMP_FLAGS} -rdynamic -lrt -Wl,--whole-archive -L${CMAKE_HOST_SYSTEM_PROCESSOR} -lcorenrnmech -L$(libdir) -lcoreneuron -Wl,--no-whole-archive"
95107
)
96108
else()
97109
set_property(GLOBAL PROPERTY CORENEURON_LIB_LINK_FLAGS

CMakeLists.txt

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -426,7 +426,12 @@ endif()
426426
# =============================================================================
427427
# Common CXX flags : ignore unknown pragma warnings
428428
# =============================================================================
429-
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${IGNORE_UNKNOWN_PRAGMA_FLAGS}")
429+
# Do not set this when building wheels. The nrnivmodl workflow means that we do not know what
430+
# compiler will be invoked with these flags, so we have to use flags that are as generic as
431+
# possible.
432+
if(NOT DEFINED NRN_WHEEL_BUILD OR NOT NRN_WHEEL_BUILD)
433+
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${IGNORE_UNKNOWN_PRAGMA_FLAGS}")
434+
endif()
430435

431436
# =============================================================================
432437
# Add main directories
@@ -436,7 +441,21 @@ add_subdirectory(coreneuron)
436441
if(CORENRN_ENABLE_GPU)
437442
get_target_property(CORENRN_LINK_LIBRARIES coreneuron INTERFACE_LINK_LIBRARIES)
438443
foreach(LIB ${CORENRN_LINK_LIBRARIES})
439-
set_property(GLOBAL APPEND_STRING PROPERTY CORENEURON_LIB_LINK_FLAGS " ${LIB}")
444+
get_filename_component(dir_path ${LIB} DIRECTORY)
445+
if(TARGET ${LIB})
446+
# See, for example, caliper where the coreneuron target depends on the caliper target (so we
447+
# get LIB=caliper in this loop), but -l and -L are already added manually here:
448+
# https://github.com/BlueBrain/CoreNeuron/blob/856cea4aa647c8f2b0d5bda6d0fc32144c5942e3/CMakeLists.txt#L411-L412
449+
message(
450+
NOTICE
451+
"Ignoring dependency '${LIB}' of 'coreneuron' and assuming relevant flags have already been added to CORENEURON_LIB_LINK_FLAGS."
452+
)
453+
elseif(NOT dir_path)
454+
# In case LIB is not a target but is just the name of a library, e.g. "dl"
455+
set_property(GLOBAL APPEND_STRING PROPERTY CORENEURON_LIB_LINK_FLAGS " -l${LIB}")
456+
else()
457+
set_property(GLOBAL APPEND_STRING PROPERTY CORENEURON_LIB_LINK_FLAGS " ${LIB}")
458+
endif()
440459
endforeach()
441460
endif()
442461

@@ -528,7 +547,6 @@ message(STATUS "COMPILE FLAGS | ${CORENRN_CXX_FLAGS}")
528547
message(STATUS "Build Type | ${COMPILE_LIBRARY_TYPE}")
529548
message(STATUS "MPI | ${CORENRN_ENABLE_MPI}")
530549
if(CORENRN_ENABLE_MPI)
531-
message(STATUS " INC | ${MPI_CXX_INCLUDE_PATH}")
532550
message(STATUS " DYNAMIC | ${CORENRN_ENABLE_MPI_DYNAMIC}")
533551
if(CORENRN_ENABLE_MPI_DYNAMIC AND NRN_MPI_LIBNAME_LIST)
534552
# ~~~
@@ -543,6 +561,8 @@ if(CORENRN_ENABLE_MPI)
543561
message(STATUS " LIBNAME | core${libname}")
544562
message(STATUS " INC | ${include}")
545563
endforeach(val)
564+
else()
565+
message(STATUS " INC | ${MPI_CXX_INCLUDE_PATH}")
546566
endif()
547567
endif()
548568
message(STATUS "OpenMP | ${CORENRN_ENABLE_OPENMP}")

extra/nrnivmodl_core_makefile.in

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,14 +52,26 @@ CORENRNLIB_FLAGS += $(if @caliper_LIB_DIR@,-L@caliper_LIB_DIR@,)
5252
# coreneuron/utils/randoms goes first because it needs to override the NEURON
5353
# directory in INCFLAGS
5454
INCLUDES = -I$(CORENRN_INC_DIR)/coreneuron/utils/randoms $(INCFLAGS) -I$(CORENRN_INC_DIR)
55-
INCLUDES += $(if @MPI_CXX_INCLUDE_PATH@, -I$(subst ;, -I,@MPI_CXX_INCLUDE_PATH@),)
55+
ifeq (@CORENRN_ENABLE_MPI_DYNAMIC@, OFF)
56+
INCLUDES += $(if @MPI_CXX_INCLUDE_PATH@, -I$(subst ;, -I,@MPI_CXX_INCLUDE_PATH@),)
57+
endif
5658
INCLUDES += $(if @reportinglib_INCLUDE_DIR@, -I$(subst ;, -I,@reportinglib_INCLUDE_DIR@),)
5759

5860
# CXX is always defined. If the definition comes from default change it
5961
ifeq ($(origin CXX), default)
6062
CXX = @CMAKE_CXX_COMPILER@
6163
endif
6264

65+
ifeq (@CORENRN_ENABLE_GPU@, ON)
66+
ifneq ($(shell $(CXX) --version | grep -o nvc++), nvc++)
67+
$(error GPU wheels are only compatible with the NVIDIA C++ compiler nvc++, but CXX=$(CXX) and --version gives $(shell $(CXX) --version))
68+
endif
69+
# nvc++ -dumpversion is simpler, but only available from 22.2
70+
ifeq ($(findstring nvc++ @CORENRN_NVHPC_MAJOR_MINOR_VERSION@, $(shell $(CXX) --version)),)
71+
$(error GPU wheels are currently not compatible across NVIDIA HPC SDK versions. You have $(shell $(CXX) -V | grep nvc++) but this wheel was built with @CORENRN_NVHPC_MAJOR_MINOR_VERSION@.)
72+
endif
73+
endif
74+
6375
# In case of wheel, python and perl exe paths are from the build machine.
6476
# First prefer env variables set by neuron's nrnivmodl wrapper then check
6577
# binary used during build. If they don't exist then simply use python and

0 commit comments

Comments
 (0)