From 726637187ac1c7cacc2883795022191af44d7047 Mon Sep 17 00:00:00 2001 From: Pramod S Kumbhar Date: Thu, 9 Sep 2021 00:03:17 +0200 Subject: [PATCH 01/11] Fixes for building portable wheel * Do not link to MPI libraries if MPI is not enabled * Use CXX compiler set on target machine e.g. via env variable * Perl and Python executable may not be exist in same path. Check for existence before using them. Todo: - [ ] Check is Perl and Python should be set via wrapper? Perl is safe. May be only python is worth. --- extra/nrnivmodl_core_makefile.in | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/extra/nrnivmodl_core_makefile.in b/extra/nrnivmodl_core_makefile.in index 7e18688ac..81c359b2a 100644 --- a/extra/nrnivmodl_core_makefile.in +++ b/extra/nrnivmodl_core_makefile.in @@ -53,8 +53,23 @@ INCLUDES = $(INCFLAGS) -I$(CORENRN_INC_DIR) -I$(CORENRN_INC_DIR)/coreneuron/util INCLUDES += $(if @MPI_CXX_INCLUDE_PATH@, -I$(subst ;, -I,@MPI_CXX_INCLUDE_PATH@),) INCLUDES += $(if @reportinglib_INCLUDE_DIR@, -I$(subst ;, -I,@reportinglib_INCLUDE_DIR@),) -# C++ compilation and link commands -CXX = @CMAKE_CXX_COMPILER@ +# CXX is always defined. If the definition comes from default change it +ifeq ($(origin CXX), default) + CXX = @CMAKE_CXX_COMPILER@ +endif + +# Python and Perl exe paths might be from build machine in case of wheel +# Use python and perl if binaries doesn't exist. +# TODO: May be wrapper script can set these as env vars? +PYTHON_EXE=@PYTHON_EXECUTABLE@ +PERL_EXE=@PERL_EXECUTABLE@ +ifeq ($(wildcard $(PYTHON_EXE)),) + PYTHON_EXE=python +endif +ifeq ($(wildcard $(PERL_EXE)),) + PERL_EXE=perl +endif + CXXFLAGS = @CORENRN_CXX_FLAGS@ CXX_COMPILE_CMD = $(CXX) $(CXXFLAGS) @CMAKE_CXX_COMPILE_OPTIONS_PIC@ @CORENRN_COMMON_COMPILE_DEFS@ $(INCLUDES) CXX_LINK_EXE_CMD = $(CXX) $(CXXFLAGS) @CMAKE_EXE_LINKER_FLAGS@ @@ -229,14 +244,14 @@ $(mod_ispc_cpp_files): $(MOD_TO_CPP_DIR)/%.cpp: $(MOD_TO_CPP_DIR)/%.ispc # generate mod registration function. Dont overwrite if it's not changed $(MOD_FUNC_CPP): build_always | $(MOD_TO_CPP_DIR) - @PERL_EXECUTABLE@ $(CORENRN_SHARE_CORENRN_DIR)/mod_func.c.pl $(mod_files_names) > $(MOD_FUNC_CPP).tmp + $(PERL_EXE) $(CORENRN_SHARE_CORENRN_DIR)/mod_func.c.pl $(mod_files_names) > $(MOD_FUNC_CPP).tmp diff -q $(MOD_FUNC_CPP).tmp $(MOD_FUNC_CPP) || \ mv $(MOD_FUNC_CPP).tmp $(MOD_FUNC_CPP) # header to avoid function callbacks using function pointers $(KINDERIV_H_PATH): $(mod_cpp_files) build_always | $(MOD_TO_CPP_DIR) cd $(MOD_TO_CPP_DIR); \ - @PYTHON_EXECUTABLE@ $(CORENRN_SHARE_CORENRN_DIR)/kinderiv.py; + $(PYTHON_EXE) $(CORENRN_SHARE_CORENRN_DIR)/kinderiv.py; # symlink to cpp files provided by coreneuron $(MOD_TO_CPP_DIR)/%.cpp: $(CORENRN_SHARE_MOD2CPP_DIR)/%.cpp | $(MOD_TO_CPP_DIR) From 18cdf998ae64c6394e117f47ea4364579f50b984 Mon Sep 17 00:00:00 2001 From: Pramod Kumbhar Date: Tue, 12 Oct 2021 23:09:10 +0200 Subject: [PATCH 02/11] neuron cmake option consistency: CORENRN_ENABLE_DYNAMIC_MPI -> CORENRN_ENABLE_MPI_DYNAMIC --- .github/workflows/coreneuron-ci.yml | 4 ++-- CMakeLists.txt | 8 ++++---- coreneuron/CMakeLists.txt | 4 ++-- coreneuron/apps/main1.cpp | 2 +- coreneuron/mpi/nrnmpi.h | 2 +- tests/CMakeLists.txt | 2 +- tests/integration/CMakeLists.txt | 2 +- 7 files changed, 12 insertions(+), 12 deletions(-) diff --git a/.github/workflows/coreneuron-ci.yml b/.github/workflows/coreneuron-ci.yml index a7adc90fb..5faceb09c 100644 --- a/.github/workflows/coreneuron-ci.yml +++ b/.github/workflows/coreneuron-ci.yml @@ -36,8 +36,8 @@ jobs: config: # Defaults: CORENRN_ENABLE_SOA=ON CORENRN_ENABLE_MPI=ON - {cmake_option: "-DCORENRN_ENABLE_MPI=ON", documentation: ON} - - {cmake_option: "-DCORENRN_ENABLE_DYNAMIC_MPI=ON"} - - {cmake_option: "-DCORENRN_ENABLE_DYNAMIC_MPI=ON -DCORENRN_ENABLE_SHARED=OFF"} + - {cmake_option: "-DCORENRN_ENABLE_MPI_DYNAMIC=ON"} + - {cmake_option: "-DCORENRN_ENABLE_MPI_DYNAMIC=ON -DCORENRN_ENABLE_SHARED=OFF"} - {cmake_option: "-DCORENRN_ENABLE_MPI=OFF"} - {cmake_option: "-DCORENRN_ENABLE_SOA=OFF"} - {use_nmodl: ON, py_version: 3.6.7} diff --git a/CMakeLists.txt b/CMakeLists.txt index 89f399d0c..5c373b20b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -88,7 +88,7 @@ option(CORENRN_ENABLE_OPENMP "Build the CORE NEURON with OpenMP implementation" option(CORENRN_ENABLE_TIMEOUT "Enable nrn_timeout implementation" ON) option(CORENRN_ENABLE_REPORTING "Enable use of ReportingLib for soma reports" OFF) option(CORENRN_ENABLE_MPI "Enable MPI-based execution" ON) -option(CORENRN_ENABLE_DYNAMIC_MPI "Enable dynamic MPI support" OFF) +option(CORENRN_ENABLE_MPI_DYNAMIC "Enable dynamic MPI support" OFF) option(CORENRN_ENABLE_SOA "Enable SoA Memory Layout" ON) option(CORENRN_ENABLE_HOC_EXP "Enable wrapping exp with hoc_exp()" OFF) option(CORENRN_ENABLE_SPLAYTREE_QUEUING "Enable use of Splay tree for spike queuing" ON) @@ -332,11 +332,11 @@ set(NMODL_ENABLE_LEGACY_UNITS ${CORENRN_ENABLE_LEGACY_UNITS} CACHE BOOL "" FORCE) -if(CORENRN_ENABLE_DYNAMIC_MPI) +if(CORENRN_ENABLE_MPI_DYNAMIC) if(NOT CORENRN_ENABLE_MPI) message(FATAL_ERROR "Cannot enable dynamic mpi without mpi") endif() - add_compile_definitions(CORENRN_ENABLE_DYNAMIC_MPI) + add_compile_definitions(CORENRN_ENABLE_MPI_DYNAMIC) endif() if(CORENRN_ENABLE_PRCELLSTATE) @@ -499,7 +499,7 @@ message(STATUS "Build Type | ${COMPILE_LIBRARY_TYPE}") message(STATUS "MPI | ${CORENRN_ENABLE_MPI}") if(CORENRN_ENABLE_MPI) message(STATUS " INC | ${MPI_CXX_INCLUDE_PATH}") - message(STATUS " DYNAMIC | ${CORENRN_ENABLE_DYNAMIC_MPI}") + message(STATUS " DYNAMIC | ${CORENRN_ENABLE_MPI_DYNAMIC}") endif() message(STATUS "OpenMP | ${CORENRN_ENABLE_OPENMP}") message(STATUS "Use legacy units | ${CORENRN_ENABLE_LEGACY_UNITS}") diff --git a/coreneuron/CMakeLists.txt b/coreneuron/CMakeLists.txt index b6e1fe567..3d920402f 100644 --- a/coreneuron/CMakeLists.txt +++ b/coreneuron/CMakeLists.txt @@ -170,7 +170,7 @@ add_custom_target(kin_deriv_header DEPENDS "${KINDERIV_HEADER_FILE}") # mpi related target, this is a separate library for dynamic MPI if(CORENRN_ENABLE_MPI) - if(CORENRN_ENABLE_DYNAMIC_MPI) + if(CORENRN_ENABLE_MPI_DYNAMIC) add_library(corenrn_mpi SHARED ${MPI_LIB_FILES}) else() add_library(corenrn_mpi OBJECT ${MPI_LIB_FILES}) @@ -192,7 +192,7 @@ add_library( ${MPI_CORE_FILES} ${OBJ_MPI}) if(CORENRN_ENABLE_MPI) - if(CORENRN_ENABLE_DYNAMIC_MPI) + if(CORENRN_ENABLE_MPI_DYNAMIC) target_link_libraries(coreneuron ${CMAKE_DL_LIBS}) target_link_libraries(corenrn_mpi ${MPI_CXX_LIBRARIES}) target_compile_definitions(coreneuron diff --git a/coreneuron/apps/main1.cpp b/coreneuron/apps/main1.cpp index a240a15f7..e578bdf27 100644 --- a/coreneuron/apps/main1.cpp +++ b/coreneuron/apps/main1.cpp @@ -473,7 +473,7 @@ extern "C" void mk_mech_init(int argc, char** argv) { #if NRNMPI if (corenrn_param.mpi_enable) { -#ifdef CORENRN_ENABLE_DYNAMIC_MPI +#ifdef CORENRN_ENABLE_MPI_DYNAMIC auto mpi_handle = load_dynamic_mpi(); mpi_manager().resolve_symbols(mpi_handle); #endif diff --git a/coreneuron/mpi/nrnmpi.h b/coreneuron/mpi/nrnmpi.h index a30caf135..44d0f564d 100644 --- a/coreneuron/mpi/nrnmpi.h +++ b/coreneuron/mpi/nrnmpi.h @@ -76,7 +76,7 @@ struct mpi_function> : mpi_function_b using mpi_function_base::mpi_function_base; template // in principle deducible from `function_ptr` auto operator()(Args&&... args) const { -#ifdef CORENRN_ENABLE_DYNAMIC_MPI +#ifdef CORENRN_ENABLE_MPI_DYNAMIC // Dynamic MPI, m_fptr should have been initialised via dlsym. assert(m_fptr); return (*reinterpret_cast(m_fptr))(std::forward( args )...); diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index a363b40d8..eababfe1f 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -31,7 +31,7 @@ if(Boost_FOUND) add_subdirectory(unit/queueing) # lfp test uses nrnmpi_* wrappers but does not load the dynamic MPI library TODO: re-enable # after NEURON and CoreNEURON dynamic MPI are merged - if(NOT CORENRN_ENABLE_DYNAMIC_MPI) + if(NOT CORENRN_ENABLE_MPI_DYNAMIC) add_subdirectory(unit/lfp) endif() endif() diff --git a/tests/integration/CMakeLists.txt b/tests/integration/CMakeLists.txt index cb3a3d905..f08ac4cda 100644 --- a/tests/integration/CMakeLists.txt +++ b/tests/integration/CMakeLists.txt @@ -151,7 +151,7 @@ if(CORENRN_ENABLE_REPORTING) endif() # DYLD_LIBRARY_PATH.LD_LIBRARY_PATH for dynamic MPI build -if(CORENRN_ENABLE_DYNAMIC_MPI) +if(CORENRN_ENABLE_MPI_DYNAMIC) set(TEST_ENV LD_LIBRARY_PATH=${PROJECT_BINARY_DIR}/lib:$ENV{LD_LIBRARY_PATH} DYLD_LIBRARY_PATH=${PROJECT_BINARY_DIR}/lib:$ENV{DYLD_LIBRARY_PATH}) set_tests_properties(${CORENRN_TEST_NAMES} PROPERTIES ENVIRONMENT "${TEST_ENV}") From c1879780c9d216c0318f3519d0275d26f94b1dae Mon Sep 17 00:00:00 2001 From: Pramod Kumbhar Date: Wed, 13 Oct 2021 01:04:49 +0200 Subject: [PATCH 03/11] Build multiple MPI libraries for dynamic MPI support * this is only supported when build as a submodule of neuron to avoid cmake code duplication * leverage -DNRN_MPI_DYNAMIC="a;b;" option of neuron * MPI distribution specific libcorenrnmpi_.so is built and installed under lib folder * TODO: still need to inrerface with NEURON side On BB5 this is tested with: ``` module load unstable cmake python hpe-mpi flex bison python-dev llvm gcc cmake .. -DCMAKE_INSTALL_PREFIX=`pwd`/install \ -DCORENRN_ENABLE_GPU=OFF -DNRN_ENABLE_INTERVIEWS=OFF -DNRN_ENABLE_RX3D=OFF \ -DNRN_ENABLE_CORENEURON=ON -DCORENRN_ENABLE_NMODL=OFF -DCORENRN_CLANG_FORMAT=ON \ -DNRN_ENABLE_MPI_DYNAMIC=ON \ -DNRN_MPI_DYNAMIC="/gpfs/bbp.cscs.ch/ssd/apps/hpc/jenkins/deploy/externals/2021-01-06/linux-rhel7-x86_64/gcc-9.3.0/intel-mpi-2019.8.254-rrc5ip/impi/2019.8.254/intel64/include/;/gpfs/bbp.cscs.ch/ssd/apps/hpc/jenkins/deploy/externals/2021-01-06/linux-rhel7-x86_64/gcc-9.3.0/hpe-mpi-2.22.hmpt-r52ypu/include;" \ -DCORENRN_CMAKE_FORMAT=ON -DPYTHON_EXECUTABLE=`which python3.8` make -j12 make install ``` --- CMakeLists.txt | 14 ++++++ coreneuron/CMakeLists.txt | 93 ++++++++++++++++++++++++++++++--------- 2 files changed, 85 insertions(+), 22 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 5c373b20b..81e97d7ab 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -500,6 +500,20 @@ message(STATUS "MPI | ${CORENRN_ENABLE_MPI}") if(CORENRN_ENABLE_MPI) message(STATUS " INC | ${MPI_CXX_INCLUDE_PATH}") message(STATUS " DYNAMIC | ${CORENRN_ENABLE_MPI_DYNAMIC}") + if(CORENRN_ENABLE_MPI_DYNAMIC) + # ~~~ + # for dynamic mpi, rely on neuron for list of libraries to build this is + # to avoid cmake code duplication on the coreneuron side + # ~~~ + list(LENGTH NRN_MPI_LIBNAME_LIST _num_mpi) + math(EXPR num_mpi "${_num_mpi} - 1") + foreach(val RANGE ${num_mpi}) + list(GET NRN_MPI_LIBNAME_LIST ${val} libname) + list(GET NRN_MPI_INCLUDE_LIST ${val} include) + message(STATUS " LIBNAME | core${libname}") + message(STATUS " INC | ${include}") + endforeach(val) + endif() endif() message(STATUS "OpenMP | ${CORENRN_ENABLE_OPENMP}") message(STATUS "Use legacy units | ${CORENRN_ENABLE_LEGACY_UNITS}") diff --git a/coreneuron/CMakeLists.txt b/coreneuron/CMakeLists.txt index 3d920402f..47bf1f682 100644 --- a/coreneuron/CMakeLists.txt +++ b/coreneuron/CMakeLists.txt @@ -168,16 +168,12 @@ add_custom_target(kin_deriv_header DEPENDS "${KINDERIV_HEADER_FILE}") # create libraries # ============================================================================= -# mpi related target, this is a separate library for dynamic MPI -if(CORENRN_ENABLE_MPI) - if(CORENRN_ENABLE_MPI_DYNAMIC) - add_library(corenrn_mpi SHARED ${MPI_LIB_FILES}) - else() - add_library(corenrn_mpi OBJECT ${MPI_LIB_FILES}) - set(OBJ_MPI $) - endif() +# for non-dynamic mpi mode just build object files +if(CORENRN_ENABLE_MPI AND NOT CORENRN_ENABLE_MPI_DYNAMIC) + add_library(corenrn_mpi OBJECT ${MPI_LIB_FILES}) target_include_directories(corenrn_mpi PRIVATE ${MPI_INCLUDE_PATH}) set_property(TARGET corenrn_mpi PROPERTY POSITION_INDEPENDENT_CODE ON) + set(CORENRN_MPI_OBJ $) endif() # main coreneuron library @@ -190,17 +186,77 @@ add_library( ${cudacorenrn_objs} ${NMODL_INBUILT_MOD_OUTPUTS} ${MPI_CORE_FILES} - ${OBJ_MPI}) -if(CORENRN_ENABLE_MPI) - if(CORENRN_ENABLE_MPI_DYNAMIC) - target_link_libraries(coreneuron ${CMAKE_DL_LIBS}) + ${CORENRN_MPI_OBJ}) + +# we can link to MPI libraries in non-dynamic-mpi build +if(CORENRN_ENABLE_MPI AND NOT CORENRN_ENABLE_MPI_DYNAMIC) + target_link_libraries(coreneuron ${MPI_CXX_LIBRARIES}) +endif() + +# this is where we handle dynamic mpi library build +if(CORENRN_ENABLE_MPI AND CORENRN_ENABLE_MPI_DYNAMIC) + # ~~~ + # main coreneuron library needs to be linked to libdl.so and + # and should be aware of shared library suffix on different platforms. + # ~~~ + target_link_libraries(coreneuron ${CMAKE_DL_LIBS}) + target_compile_definitions(coreneuron + PUBLIC CORENRN_SHARED_LIBRARY_SUFFIX=${CMAKE_SHARED_LIBRARY_SUFFIX}) + + # store mpi library targets that will be built + list(APPEND corenrn_mpi_targets "") + + # ~~~ + # if coreneuron is built as a submodule of neuron then check if NEURON has created + # list of libraries that needs to be built. We use neuron cmake variables here because + # we don't need to duplicate CMake code into coreneuron (we want to have unified cmake + # project soon). In the absense of neuron just build a single library corenrn_mpi. + # This is mostly used for the testing. Should this be removed and make dynamic mpi + # only usable via neuron build? + # ~~~ + if(NOT CORENEURON_AS_SUBPROJECT) + add_library(corenrn_mpi SHARED ${MPI_LIB_FILES}) target_link_libraries(corenrn_mpi ${MPI_CXX_LIBRARIES}) - target_compile_definitions(coreneuron - PUBLIC CORENRN_SHARED_LIBRARY_SUFFIX=${CMAKE_SHARED_LIBRARY_SUFFIX}) + target_include_directories(corenrn_mpi PRIVATE ${MPI_INCLUDE_PATH}) + set_property(TARGET corenrn_mpi PROPERTY POSITION_INDEPENDENT_CODE ON) + list(APPEND corenrn_mpi_targets "corenrn_mpi") else() - target_link_libraries(coreneuron ${MPI_CXX_LIBRARIES}) + # ~~~ + # from neuron we know how many different libraries needs to be built, their names + # include paths to be used for building shared libraries. Iterate through those + # and build separate library for each MPI distribution. + # ~~~ + list(LENGTH NRN_MPI_LIBNAME_LIST _num_mpi) + math(EXPR num_mpi "${_num_mpi} - 1") + foreach(val RANGE ${num_mpi}) + list(GET NRN_MPI_INCLUDE_LIST ${val} include) + list(GET NRN_MPI_LIBNAME_LIST ${val} libname) + + add_library(core${libname}_lib SHARED ${MPI_LIB_FILES}) + target_include_directories(core${libname}_lib PUBLIC ${include}) + + # ~~~ + # TODO: somehow mingw requires explicit linking. This needs to be verified + # when we will test coreneuron on windows. + # ~~~ + if(MINGW) # type msmpi only + add_dependencies(core${libname}_lib coreneuron) + target_link_libraries(core${libname}_lib ${MPI_C_LIBRARIES}) + target_link_libraries(core${libname}_lib coreneuron) + endif() + set_property(TARGET core${libname}_lib PROPERTY OUTPUT_NAME core${libname}) + list(APPEND corenrn_mpi_targets "core${libname}_lib") + endforeach(val) endif() + + set_target_properties( + ${corenrn_mpi_targets} + PROPERTIES ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib + LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib + POSITION_INDEPENDENT_CODE ON) + install(TARGETS ${corenrn_mpi_targets} DESTINATION lib) endif() + # Prevent CMake from running a device code link step when assembling libcoreneuron.a in GPU builds. # The device code linking needs to be deferred to the final step, where it is done by `nvc++ -cuda`. set_target_properties(coreneuron PROPERTIES CUDA_SEPARABLE_COMPILATION ON) @@ -222,13 +278,6 @@ target_include_directories(coreneuron SYSTEM target_include_directories(coreneuron SYSTEM PRIVATE ${CORENEURON_PROJECT_SOURCE_DIR}/external/CLI11/include) -if(CORENRN_ENABLE_MPI) - set_target_properties( - corenrn_mpi - PROPERTIES ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib - LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib - POSITION_INDEPENDENT_CODE ON) -endif() set_target_properties( coreneuron scopmath PROPERTIES ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib From 84c14d3d4718ab3495ed5d281bde10f2041d0c70 Mon Sep 17 00:00:00 2001 From: Pramod Kumbhar Date: Wed, 13 Oct 2021 17:19:23 +0200 Subject: [PATCH 04/11] Connect dynamic MPI with neuron * add new CLI option --corenrn-mpi-lib * load & resolve symbols only once * is_quite now passed as argument to nrnmpi_init * remove usage of [dy]ld_library_path --- coreneuron/apps/corenrn_parameters.cpp | 5 +++ coreneuron/apps/corenrn_parameters.hpp | 1 + coreneuron/apps/main1.cpp | 40 +++++++++++++++++++----- coreneuron/mechanism/mech/enginemech.cpp | 9 ++++-- coreneuron/mpi/lib/nrnmpi.cpp | 4 +-- coreneuron/mpi/nrnmpidec.h | 2 +- tests/integration/CMakeLists.txt | 25 ++++++++++----- tests/unit/lfp/lfp.cpp | 2 +- 8 files changed, 67 insertions(+), 21 deletions(-) diff --git a/coreneuron/apps/corenrn_parameters.cpp b/coreneuron/apps/corenrn_parameters.cpp index f2ea614eb..f7cfed7b3 100644 --- a/coreneuron/apps/corenrn_parameters.cpp +++ b/coreneuron/apps/corenrn_parameters.cpp @@ -25,6 +25,10 @@ corenrn_parameters::corenrn_parameters() { "--mpi", this->mpi_enable, "Enable MPI. In order to initialize MPI environment this argument must be specified."); + app.add_option("--corenrn-mpi-lib", + this->corenrn_mpi_lib, + "CoreNEURON MPI library to load for dynamic MPI support", + false); app.add_flag("--gpu", this->gpu, "Activate GPU computation."); app.add_option("--dt", this->dt, @@ -187,6 +191,7 @@ void corenrn_parameters::parse(int argc, char** argv) { std::ostream& operator<<(std::ostream& os, const corenrn_parameters& corenrn_param) { os << "GENERAL PARAMETERS" << std::endl << "--mpi=" << (corenrn_param.mpi_enable ? "true" : "false") << std::endl + << "--corenrn-mpi-lib=" << corenrn_param.corenrn_mpi_lib << std::endl << "--gpu=" << (corenrn_param.gpu ? "true" : "false") << std::endl << "--dt=" << corenrn_param.dt << std::endl << "--tstop=" << corenrn_param.tstop << std::endl diff --git a/coreneuron/apps/corenrn_parameters.hpp b/coreneuron/apps/corenrn_parameters.hpp index 781fceefc..03bb72254 100644 --- a/coreneuron/apps/corenrn_parameters.hpp +++ b/coreneuron/apps/corenrn_parameters.hpp @@ -52,6 +52,7 @@ struct corenrn_parameters { int seed = -1; /// Initialization seed for random number generator (int) bool mpi_enable = false; /// Enable MPI flag. + std::string corenrn_mpi_lib; /// Name of CoreNEURON MPI library to load dynamically. bool skip_mpi_finalize = false; /// Skip MPI finalization bool multisend = false; /// Use Multisend spike exchange instead of Allgather. bool threading = false; /// Enable pthread/openmp diff --git a/coreneuron/apps/main1.cpp b/coreneuron/apps/main1.cpp index e578bdf27..aeae1fbea 100644 --- a/coreneuron/apps/main1.cpp +++ b/coreneuron/apps/main1.cpp @@ -83,7 +83,11 @@ void set_openmp_threads(int nthread) { * Convert char* containing arguments from neuron to char* argv[] for * coreneuron command line argument parser. */ -char* prepare_args(int& argc, char**& argv, int use_mpi, const char* arg) { +char* prepare_args(int& argc, + char**& argv, + int use_mpi, + const char* corenrn_mpi_lib, + const char* arg) { // first construct all arguments as string std::string args(arg); args.insert(0, " coreneuron "); @@ -92,6 +96,14 @@ char* prepare_args(int& argc, char**& argv, int use_mpi, const char* arg) { args.append(" --mpi "); } + // if neuron has passed name of MPI library then add it to CLI + std::string mpi_lib(corenrn_mpi_lib); + if (!mpi_lib.empty()) { + args.append(" --corenrn-mpi-lib "); + mpi_lib += " "; + args.append(mpi_lib); + } + // we can't modify string with strtok, make copy char* first = strdup(args.c_str()); const char* sep = " "; @@ -454,10 +466,9 @@ using namespace coreneuron; #if NRNMPI #define STRINGIFY(x) #x #define TOSTRING(x) STRINGIFY(x) -static void* load_dynamic_mpi() { +static void* load_dynamic_mpi(const std::string& libname) { dlerror(); - void* handle = dlopen("libcorenrn_mpi" TOSTRING(CORENRN_SHARED_LIBRARY_SUFFIX), - RTLD_NOW | RTLD_GLOBAL); + void* handle = dlopen(libname.c_str(), RTLD_NOW | RTLD_GLOBAL); const char* error = dlerror(); if (error) { std::string err_msg = std::string("Could not open dynamic MPI library: ") + error + "\n"; @@ -474,10 +485,25 @@ extern "C" void mk_mech_init(int argc, char** argv) { #if NRNMPI if (corenrn_param.mpi_enable) { #ifdef CORENRN_ENABLE_MPI_DYNAMIC - auto mpi_handle = load_dynamic_mpi(); - mpi_manager().resolve_symbols(mpi_handle); + // coreneuron rely on neuron to detect mpi library distribution and + // the name of library itself. Make sure the library name is specified + // via CLI option. + if (corenrn_param.corenrn_mpi_lib.empty()) { + throw std::runtime_error( + "For dynamic MPI support you must pass '--corenrn-mpi-lib " + "/path/libcorenrnmpi_.` argument\n"); + } + + // neuron can call coreneuron multiple times and hence we do not + // want to initialize/load mpi library multiple times + static bool mpi_lib_loaded = false; + if (!mpi_lib_loaded) { + auto mpi_handle = load_dynamic_mpi(corenrn_param.corenrn_mpi_lib); + mpi_manager().resolve_symbols(mpi_handle); + mpi_lib_loaded = true; + } #endif - auto ret = nrnmpi_init(&argc, &argv); + auto ret = nrnmpi_init(&argc, &argv, corenrn_param.is_quiet()); nrnmpi_numprocs = ret.numprocs; nrnmpi_myid = ret.myid; } diff --git a/coreneuron/mechanism/mech/enginemech.cpp b/coreneuron/mechanism/mech/enginemech.cpp index 0c645c58b..a42f0dd2b 100644 --- a/coreneuron/mechanism/mech/enginemech.cpp +++ b/coreneuron/mechanism/mech/enginemech.cpp @@ -58,7 +58,11 @@ extern bool corenrn_embedded; extern int corenrn_embedded_nthread; /// parse arguments from neuron and prepare new one for coreneuron -char* prepare_args(int& argc, char**& argv, int use_mpi, const char* nrn_arg); +char* prepare_args(int& argc, + char**& argv, + int use_mpi, + const char* corenrn_mpi_lib, + const char* nrn_arg); /// initialize standard mechanisms from coreneuron void mk_mech_init(int argc, char** argv); @@ -80,6 +84,7 @@ int corenrn_embedded_run(int nthread, int have_gaps, int use_mpi, int use_fast_imem, + const char* corenrn_mpi_lib, const char* nrn_arg) { // set coreneuron's internal variable based on neuron arguments corenrn_embedded = true; @@ -93,7 +98,7 @@ int corenrn_embedded_run(int nthread, // pre-process argumnets from neuron and prepare new for coreneuron int argc; char** argv; - char* new_arg = prepare_args(argc, argv, use_mpi, nrn_arg); + char* new_arg = prepare_args(argc, argv, use_mpi, corenrn_mpi_lib, nrn_arg); // initialize internal arguments mk_mech_init(argc, argv); diff --git a/coreneuron/mpi/lib/nrnmpi.cpp b/coreneuron/mpi/lib/nrnmpi.cpp index 591abe348..070ce05fd 100644 --- a/coreneuron/mpi/lib/nrnmpi.cpp +++ b/coreneuron/mpi/lib/nrnmpi.cpp @@ -34,7 +34,7 @@ static void nrn_fatal_error(const char* msg) { nrnmpi_abort_impl(-1); } -nrnmpi_init_ret_t nrnmpi_init_impl(int* pargc, char*** pargv) { +nrnmpi_init_ret_t nrnmpi_init_impl(int* pargc, char*** pargv, bool is_quiet) { nrnmpi_under_nrncontrol_ = true; if (!nrnmpi_initialized_impl()) { @@ -54,7 +54,7 @@ nrnmpi_init_ret_t nrnmpi_init_impl(int* pargc, char*** pargv) { nrn_assert(MPI_Comm_size(nrnmpi_world_comm, &nrnmpi_numprocs_) == MPI_SUCCESS); nrnmpi_spike_initialize(); - if (nrnmpi_myid_ == 0) { + if (nrnmpi_myid_ == 0 && !is_quiet) { #if defined(_OPENMP) printf(" num_mpi=%d\n num_omp_thread=%d\n\n", nrnmpi_numprocs_, omp_get_max_threads()); #else diff --git a/coreneuron/mpi/nrnmpidec.h b/coreneuron/mpi/nrnmpidec.h index fd2747f2d..df88eaa5c 100644 --- a/coreneuron/mpi/nrnmpidec.h +++ b/coreneuron/mpi/nrnmpidec.h @@ -21,7 +21,7 @@ struct nrnmpi_init_ret_t { int numprocs; int myid; }; -extern "C" nrnmpi_init_ret_t nrnmpi_init_impl(int* pargc, char*** pargv); +extern "C" nrnmpi_init_ret_t nrnmpi_init_impl(int* pargc, char*** pargv, bool is_quiet); extern mpi_function nrnmpi_init; extern "C" void nrnmpi_finalize_impl(void); extern mpi_function nrnmpi_finalize; diff --git a/tests/integration/CMakeLists.txt b/tests/integration/CMakeLists.txt index f08ac4cda..d46a8b528 100644 --- a/tests/integration/CMakeLists.txt +++ b/tests/integration/CMakeLists.txt @@ -4,7 +4,23 @@ # See top-level LICENSE file for details. # ============================================================================= -set(COMMON_ARGS "--tstop 100. --celsius 6.3 --mpi") +if(CORENRN_ENABLE_MPI_DYNAMIC) + # ~~~ + # In case of submodule building we don't know the MPI launcher and mpi + # distribution being used. So for now just skip these tests and rely on + # neuron to test dynamic mpi mode. For coreneuron build assume are just + # building single generic mpi library libcorenrn_mpi. + # ~~~ + if(CORENEURON_AS_SUBPROJECT) + message(INFO "CoreNEURON integration tests are disabled with dynamic MPI") + return() + else() + set(CORENRN_MPI_LIB_ARG + "--corenrn-mpi-lib ${PROJECT_BINARY_DIR}/lib/libcorenrn_mpi${CMAKE_SHARED_LIBRARY_SUFFIX}") + endif() +endif() + +set(COMMON_ARGS "--tstop 100. --celsius 6.3 --mpi ${CORENRN_MPI_LIB_ARG}") set(MODEL_STATS_ARG "--model-stats") set(RING_DATASET_DIR "${CMAKE_CURRENT_SOURCE_DIR}/ring") set(RING_COMMON_ARGS "--datpath ${RING_DATASET_DIR} ${COMMON_ARGS}") @@ -149,10 +165,3 @@ if(CORENRN_ENABLE_REPORTING) list(APPEND CORENRN_TEST_NAMES ${SIM_NAME}) endforeach() endif() - -# DYLD_LIBRARY_PATH.LD_LIBRARY_PATH for dynamic MPI build -if(CORENRN_ENABLE_MPI_DYNAMIC) - set(TEST_ENV LD_LIBRARY_PATH=${PROJECT_BINARY_DIR}/lib:$ENV{LD_LIBRARY_PATH} - DYLD_LIBRARY_PATH=${PROJECT_BINARY_DIR}/lib:$ENV{DYLD_LIBRARY_PATH}) - set_tests_properties(${CORENRN_TEST_NAMES} PROPERTIES ENVIRONMENT "${TEST_ENV}") -endif() diff --git a/tests/unit/lfp/lfp.cpp b/tests/unit/lfp/lfp.cpp index b65a378a0..7f5fa48ae 100644 --- a/tests/unit/lfp/lfp.cpp +++ b/tests/unit/lfp/lfp.cpp @@ -24,7 +24,7 @@ double integral(F f, double a, double b, int n) { BOOST_AUTO_TEST_CASE(LFP_PointSource_LineSource) { #if NRNMPI - nrnmpi_init(nullptr, nullptr); + nrnmpi_init(nullptr, nullptr, false); #endif double segment_length{1.0e-6}; double segment_start_val{1.0e-6}; From ed629af02c16a2ec0938bdfdbaaf4d68d41c6419 Mon Sep 17 00:00:00 2001 From: Pramod Kumbhar Date: Wed, 13 Oct 2021 20:28:35 +0200 Subject: [PATCH 05/11] Respect CORENRN_PERLEXE and CORENRN_PYTHONEXE env vars set by neuron's wheel wrapper --- extra/nrnivmodl_core_makefile.in | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/extra/nrnivmodl_core_makefile.in b/extra/nrnivmodl_core_makefile.in index 81c359b2a..5bd424865 100644 --- a/extra/nrnivmodl_core_makefile.in +++ b/extra/nrnivmodl_core_makefile.in @@ -58,16 +58,17 @@ ifeq ($(origin CXX), default) CXX = @CMAKE_CXX_COMPILER@ endif -# Python and Perl exe paths might be from build machine in case of wheel -# Use python and perl if binaries doesn't exist. -# TODO: May be wrapper script can set these as env vars? -PYTHON_EXE=@PYTHON_EXECUTABLE@ -PERL_EXE=@PERL_EXECUTABLE@ -ifeq ($(wildcard $(PYTHON_EXE)),) - PYTHON_EXE=python +# In case of wheel, python and perl exe paths are from the build machine. +# First prefer env variables set by neuron's nrnivmodl wrapper then check +# binary used during build. If they don't exist then simply use python and +# perl as the name of binaries. +CORENRN_PYTHONEXE ?= @PYTHON_EXECUTABLE@ +CORENRN_PERLEXE ?= @PERL_EXECUTABLE@ +ifeq ($(wildcard $(CORENRN_PYTHONEXE)),) + CORENRN_PYTHONEXE=python endif -ifeq ($(wildcard $(PERL_EXE)),) - PERL_EXE=perl +ifeq ($(wildcard $(CORENRN_PERLEXE)),) + CORENRN_PERLEXE=perl endif CXXFLAGS = @CORENRN_CXX_FLAGS@ @@ -244,14 +245,14 @@ $(mod_ispc_cpp_files): $(MOD_TO_CPP_DIR)/%.cpp: $(MOD_TO_CPP_DIR)/%.ispc # generate mod registration function. Dont overwrite if it's not changed $(MOD_FUNC_CPP): build_always | $(MOD_TO_CPP_DIR) - $(PERL_EXE) $(CORENRN_SHARE_CORENRN_DIR)/mod_func.c.pl $(mod_files_names) > $(MOD_FUNC_CPP).tmp + $(CORENRN_PERLEXE) $(CORENRN_SHARE_CORENRN_DIR)/mod_func.c.pl $(mod_files_names) > $(MOD_FUNC_CPP).tmp diff -q $(MOD_FUNC_CPP).tmp $(MOD_FUNC_CPP) || \ mv $(MOD_FUNC_CPP).tmp $(MOD_FUNC_CPP) # header to avoid function callbacks using function pointers $(KINDERIV_H_PATH): $(mod_cpp_files) build_always | $(MOD_TO_CPP_DIR) cd $(MOD_TO_CPP_DIR); \ - $(PYTHON_EXE) $(CORENRN_SHARE_CORENRN_DIR)/kinderiv.py; + $(CORENRN_PYTHONEXE) $(CORENRN_SHARE_CORENRN_DIR)/kinderiv.py; # symlink to cpp files provided by coreneuron $(MOD_TO_CPP_DIR)/%.cpp: $(CORENRN_SHARE_MOD2CPP_DIR)/%.cpp | $(MOD_TO_CPP_DIR) From 8df95dbde0e35a2e62261a484ee68c8b74bcfd77 Mon Sep 17 00:00:00 2001 From: Pramod S Kumbhar Date: Wed, 13 Oct 2021 23:53:47 +0200 Subject: [PATCH 06/11] Cleanup and address review comments * change --corenrn-mpi-lib to --mpi-lib * use variable for name of the library and targets --- CMakeLists.txt | 4 +-- coreneuron/CMakeLists.txt | 34 +++++++++++++++--------- coreneuron/apps/corenrn_parameters.cpp | 6 ++--- coreneuron/apps/corenrn_parameters.hpp | 2 +- coreneuron/apps/main1.cpp | 26 ++++++++---------- coreneuron/mechanism/mech/enginemech.cpp | 6 ++--- tests/integration/CMakeLists.txt | 3 ++- 7 files changed, 43 insertions(+), 38 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 81e97d7ab..2ab6e1b10 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -502,8 +502,8 @@ if(CORENRN_ENABLE_MPI) message(STATUS " DYNAMIC | ${CORENRN_ENABLE_MPI_DYNAMIC}") if(CORENRN_ENABLE_MPI_DYNAMIC) # ~~~ - # for dynamic mpi, rely on neuron for list of libraries to build this is - # to avoid cmake code duplication on the coreneuron side + # for dynamic mpi, rely on neuron for list of libraries to build + # this is to avoid cmake code duplication on the coreneuron side # ~~~ list(LENGTH NRN_MPI_LIBNAME_LIST _num_mpi) math(EXPR num_mpi "${_num_mpi} - 1") diff --git a/coreneuron/CMakeLists.txt b/coreneuron/CMakeLists.txt index 47bf1f682..825d65390 100644 --- a/coreneuron/CMakeLists.txt +++ b/coreneuron/CMakeLists.txt @@ -168,12 +168,17 @@ add_custom_target(kin_deriv_header DEPENDS "${KINDERIV_HEADER_FILE}") # create libraries # ============================================================================= +# name of coreneuron mpi objects or dynamic library +set(CORENRN_MPI_LIB_NAME + "corenrn_mpi" + CACHE INTERNAL "") + # for non-dynamic mpi mode just build object files if(CORENRN_ENABLE_MPI AND NOT CORENRN_ENABLE_MPI_DYNAMIC) - add_library(corenrn_mpi OBJECT ${MPI_LIB_FILES}) - target_include_directories(corenrn_mpi PRIVATE ${MPI_INCLUDE_PATH}) - set_property(TARGET corenrn_mpi PROPERTY POSITION_INDEPENDENT_CODE ON) - set(CORENRN_MPI_OBJ $) + add_library(${CORENRN_MPI_LIB_NAME} OBJECT ${MPI_LIB_FILES}) + target_include_directories(${CORENRN_MPI_LIB_NAME} PRIVATE ${MPI_INCLUDE_PATH}) + set_property(TARGET ${CORENRN_MPI_LIB_NAME} PROPERTY POSITION_INDEPENDENT_CODE ON) + set(CORENRN_MPI_OBJ $) endif() # main coreneuron library @@ -210,21 +215,24 @@ if(CORENRN_ENABLE_MPI AND CORENRN_ENABLE_MPI_DYNAMIC) # if coreneuron is built as a submodule of neuron then check if NEURON has created # list of libraries that needs to be built. We use neuron cmake variables here because # we don't need to duplicate CMake code into coreneuron (we want to have unified cmake - # project soon). In the absense of neuron just build a single library corenrn_mpi. - # This is mostly used for the testing. Should this be removed and make dynamic mpi - # only usable via neuron build? + # project soon). In the absense of neuron just build a single library libcorenrn_mpi. + # This is mostly used for the testing. # ~~~ if(NOT CORENEURON_AS_SUBPROJECT) - add_library(corenrn_mpi SHARED ${MPI_LIB_FILES}) - target_link_libraries(corenrn_mpi ${MPI_CXX_LIBRARIES}) - target_include_directories(corenrn_mpi PRIVATE ${MPI_INCLUDE_PATH}) - set_property(TARGET corenrn_mpi PROPERTY POSITION_INDEPENDENT_CODE ON) - list(APPEND corenrn_mpi_targets "corenrn_mpi") + add_library(${CORENRN_MPI_LIB_NAME} SHARED ${MPI_LIB_FILES}) + target_link_libraries(${CORENRN_MPI_LIB_NAME} ${MPI_CXX_LIBRARIES}) + target_include_directories(${CORENRN_MPI_LIB_NAME} PRIVATE ${MPI_INCLUDE_PATH}) + set_property(TARGET ${CORENRN_MPI_LIB_NAME} PROPERTY POSITION_INDEPENDENT_CODE ON) + list(APPEND corenrn_mpi_targets ${CORENRN_MPI_LIB_NAME}) else() # ~~~ # from neuron we know how many different libraries needs to be built, their names # include paths to be used for building shared libraries. Iterate through those - # and build separate library for each MPI distribution. + # and build separate library for each MPI distribution. For example, following + # libraries are created: + # - libcorenrn_mpich.so + # - libcorenrn_ompi.so + # - libcorenrn_mpt.so # ~~~ list(LENGTH NRN_MPI_LIBNAME_LIST _num_mpi) math(EXPR num_mpi "${_num_mpi} - 1") diff --git a/coreneuron/apps/corenrn_parameters.cpp b/coreneuron/apps/corenrn_parameters.cpp index f7cfed7b3..cd923a709 100644 --- a/coreneuron/apps/corenrn_parameters.cpp +++ b/coreneuron/apps/corenrn_parameters.cpp @@ -25,8 +25,8 @@ corenrn_parameters::corenrn_parameters() { "--mpi", this->mpi_enable, "Enable MPI. In order to initialize MPI environment this argument must be specified."); - app.add_option("--corenrn-mpi-lib", - this->corenrn_mpi_lib, + app.add_option("--mpi-lib", + this->mpi_lib, "CoreNEURON MPI library to load for dynamic MPI support", false); app.add_flag("--gpu", this->gpu, "Activate GPU computation."); @@ -191,7 +191,7 @@ void corenrn_parameters::parse(int argc, char** argv) { std::ostream& operator<<(std::ostream& os, const corenrn_parameters& corenrn_param) { os << "GENERAL PARAMETERS" << std::endl << "--mpi=" << (corenrn_param.mpi_enable ? "true" : "false") << std::endl - << "--corenrn-mpi-lib=" << corenrn_param.corenrn_mpi_lib << std::endl + << "--mpi-lib=" << corenrn_param.mpi_lib << std::endl << "--gpu=" << (corenrn_param.gpu ? "true" : "false") << std::endl << "--dt=" << corenrn_param.dt << std::endl << "--tstop=" << corenrn_param.tstop << std::endl diff --git a/coreneuron/apps/corenrn_parameters.hpp b/coreneuron/apps/corenrn_parameters.hpp index 03bb72254..5ad75eeeb 100644 --- a/coreneuron/apps/corenrn_parameters.hpp +++ b/coreneuron/apps/corenrn_parameters.hpp @@ -52,7 +52,7 @@ struct corenrn_parameters { int seed = -1; /// Initialization seed for random number generator (int) bool mpi_enable = false; /// Enable MPI flag. - std::string corenrn_mpi_lib; /// Name of CoreNEURON MPI library to load dynamically. + std::string mpi_lib; /// Name of CoreNEURON MPI library to load dynamically. bool skip_mpi_finalize = false; /// Skip MPI finalization bool multisend = false; /// Use Multisend spike exchange instead of Allgather. bool threading = false; /// Enable pthread/openmp diff --git a/coreneuron/apps/main1.cpp b/coreneuron/apps/main1.cpp index aeae1fbea..6fba311c3 100644 --- a/coreneuron/apps/main1.cpp +++ b/coreneuron/apps/main1.cpp @@ -83,11 +83,7 @@ void set_openmp_threads(int nthread) { * Convert char* containing arguments from neuron to char* argv[] for * coreneuron command line argument parser. */ -char* prepare_args(int& argc, - char**& argv, - int use_mpi, - const char* corenrn_mpi_lib, - const char* arg) { +char* prepare_args(int& argc, char**& argv, int use_mpi, const char* mpi_lib, const char* arg) { // first construct all arguments as string std::string args(arg); args.insert(0, " coreneuron "); @@ -97,11 +93,11 @@ char* prepare_args(int& argc, } // if neuron has passed name of MPI library then add it to CLI - std::string mpi_lib(corenrn_mpi_lib); - if (!mpi_lib.empty()) { - args.append(" --corenrn-mpi-lib "); - mpi_lib += " "; - args.append(mpi_lib); + std::string corenrn_mpi_lib(mpi_lib); + if (!corenrn_mpi_lib.empty()) { + args.append(" --mpi-lib "); + corenrn_mpi_lib += " "; + args.append(corenrn_mpi_lib); } // we can't modify string with strtok, make copy @@ -486,19 +482,19 @@ extern "C" void mk_mech_init(int argc, char** argv) { if (corenrn_param.mpi_enable) { #ifdef CORENRN_ENABLE_MPI_DYNAMIC // coreneuron rely on neuron to detect mpi library distribution and - // the name of library itself. Make sure the library name is specified + // the name of the library itself. Make sure the library name is specified // via CLI option. - if (corenrn_param.corenrn_mpi_lib.empty()) { + if (corenrn_param.mpi_lib.empty()) { throw std::runtime_error( - "For dynamic MPI support you must pass '--corenrn-mpi-lib " - "/path/libcorenrnmpi_.` argument\n"); + "For dynamic MPI support you must pass '--mpi-lib " + "/path/libcorenrnmpi_.` argument!\n"); } // neuron can call coreneuron multiple times and hence we do not // want to initialize/load mpi library multiple times static bool mpi_lib_loaded = false; if (!mpi_lib_loaded) { - auto mpi_handle = load_dynamic_mpi(corenrn_param.corenrn_mpi_lib); + auto mpi_handle = load_dynamic_mpi(corenrn_param.mpi_lib); mpi_manager().resolve_symbols(mpi_handle); mpi_lib_loaded = true; } diff --git a/coreneuron/mechanism/mech/enginemech.cpp b/coreneuron/mechanism/mech/enginemech.cpp index a42f0dd2b..9f09498f4 100644 --- a/coreneuron/mechanism/mech/enginemech.cpp +++ b/coreneuron/mechanism/mech/enginemech.cpp @@ -61,7 +61,7 @@ extern int corenrn_embedded_nthread; char* prepare_args(int& argc, char**& argv, int use_mpi, - const char* corenrn_mpi_lib, + const char* mpi_lib, const char* nrn_arg); /// initialize standard mechanisms from coreneuron @@ -84,7 +84,7 @@ int corenrn_embedded_run(int nthread, int have_gaps, int use_mpi, int use_fast_imem, - const char* corenrn_mpi_lib, + const char* mpi_lib, const char* nrn_arg) { // set coreneuron's internal variable based on neuron arguments corenrn_embedded = true; @@ -98,7 +98,7 @@ int corenrn_embedded_run(int nthread, // pre-process argumnets from neuron and prepare new for coreneuron int argc; char** argv; - char* new_arg = prepare_args(argc, argv, use_mpi, corenrn_mpi_lib, nrn_arg); + char* new_arg = prepare_args(argc, argv, use_mpi, mpi_lib, nrn_arg); // initialize internal arguments mk_mech_init(argc, argv); diff --git a/tests/integration/CMakeLists.txt b/tests/integration/CMakeLists.txt index d46a8b528..e07a4ebc4 100644 --- a/tests/integration/CMakeLists.txt +++ b/tests/integration/CMakeLists.txt @@ -16,7 +16,8 @@ if(CORENRN_ENABLE_MPI_DYNAMIC) return() else() set(CORENRN_MPI_LIB_ARG - "--corenrn-mpi-lib ${PROJECT_BINARY_DIR}/lib/libcorenrn_mpi${CMAKE_SHARED_LIBRARY_SUFFIX}") + "--mpi-lib ${PROJECT_BINARY_DIR}/lib/lib${CORENRN_MPI_LIB_NAME}${CMAKE_SHARED_LIBRARY_SUFFIX}" + ) endif() endif() From d8d2c31abfe82323335b8ee44922eb66546fd7c8 Mon Sep 17 00:00:00 2001 From: Pramod S Kumbhar Date: Thu, 14 Oct 2021 21:03:06 +0200 Subject: [PATCH 07/11] Fixes for segfault at finalise with dynamic MPI * do not compile mpi library with -acc -gpu flags because it results into nasty segfault * to achieve this, do not add OpenACC flags to CMAKE_CXX_FLAGS instead use targe_compile_options() * Profile interface needs to be updated because some cpp files like for mpi library may not have cuda headers because they no longer use -acc flag to compile. Check existance of ACC or CUDA active. --- CMake/MakefileBuildOptions.cmake | 2 +- CMake/OpenAccHelper.cmake | 4 +++- CMakeLists.txt | 2 +- coreneuron/CMakeLists.txt | 6 ++++++ coreneuron/utils/profile/profiler_interface.h | 6 +++--- 5 files changed, 14 insertions(+), 6 deletions(-) diff --git a/CMake/MakefileBuildOptions.cmake b/CMake/MakefileBuildOptions.cmake index 9e43e41e2..c272aab1d 100644 --- a/CMake/MakefileBuildOptions.cmake +++ b/CMake/MakefileBuildOptions.cmake @@ -73,7 +73,7 @@ endforeach() string(REPLACE ";" " " CXX14_STD_FLAGS "${CMAKE_CXX14_STANDARD_COMPILE_OPTION}") string(TOUPPER "${CMAKE_BUILD_TYPE}" _BUILD_TYPE) set(CORENRN_CXX_FLAGS - "${CMAKE_CXX_FLAGS} ${CMAKE_CXX_FLAGS_${_BUILD_TYPE}} ${CXX14_STD_FLAGS} ${NVHPC_CXX_INLINE_FLAGS}" + "${CORENRN_ACC_FLAGS} ${CMAKE_CXX_FLAGS} ${CMAKE_CXX_FLAGS_${_BUILD_TYPE}} ${CXX14_STD_FLAGS} ${NVHPC_CXX_INLINE_FLAGS}" ) # ============================================================================= diff --git a/CMake/OpenAccHelper.cmake b/CMake/OpenAccHelper.cmake index 85d28fd51..ea55d9804 100644 --- a/CMake/OpenAccHelper.cmake +++ b/CMake/OpenAccHelper.cmake @@ -65,7 +65,9 @@ if(CORENRN_ENABLE_GPU) endforeach() # avoid PGI adding standard compliant "-A" flags set(CMAKE_CXX14_STANDARD_COMPILE_OPTION --c++14) - string(APPEND CMAKE_CXX_FLAGS " ${NVHPC_ACC_COMP_FLAGS} ${PGI_DIAG_FLAGS}") + set(CORENRN_ACC_FLAGS + "${NVHPC_ACC_COMP_FLAGS} ${PGI_DIAG_FLAGS}" + CACHE INTERNAL "") string(APPEND CMAKE_EXE_LINKER_FLAGS " ${NVHPC_ACC_LINK_FLAGS}") # Use `-Mautoinline` option to compile .cpp files generated from .mod files only. This is # especially needed when we compile with -O0 or -O1 optimisation level where we get link errors. diff --git a/CMakeLists.txt b/CMakeLists.txt index 2ab6e1b10..eb07e8af8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -500,7 +500,7 @@ message(STATUS "MPI | ${CORENRN_ENABLE_MPI}") if(CORENRN_ENABLE_MPI) message(STATUS " INC | ${MPI_CXX_INCLUDE_PATH}") message(STATUS " DYNAMIC | ${CORENRN_ENABLE_MPI_DYNAMIC}") - if(CORENRN_ENABLE_MPI_DYNAMIC) + if(CORENRN_ENABLE_MPI_DYNAMIC AND NRN_MPI_LIBNAME_LIST) # ~~~ # for dynamic mpi, rely on neuron for list of libraries to build # this is to avoid cmake code duplication on the coreneuron side diff --git a/coreneuron/CMakeLists.txt b/coreneuron/CMakeLists.txt index 825d65390..f07b900af 100644 --- a/coreneuron/CMakeLists.txt +++ b/coreneuron/CMakeLists.txt @@ -310,6 +310,12 @@ add_custom_target(nrniv-core ALL DEPENDS ${output_binaries}) include_directories(${CORENEURON_PROJECT_SOURCE_DIR}) +if(CORENRN_ENABLE_GPU) + separate_arguments(CORENRN_ACC_FLAGS UNIX_COMMAND "${CORENRN_ACC_FLAGS}") + target_compile_options(coreneuron PRIVATE $<$:${CORENRN_ACC_FLAGS}>) + target_compile_options(scopmath PRIVATE $<$:${CORENRN_ACC_FLAGS}>) +endif() + # ============================================================================= # Extract link definitions to be used with nrnivmodl-core # ============================================================================= diff --git a/coreneuron/utils/profile/profiler_interface.h b/coreneuron/utils/profile/profiler_interface.h index 521c8c0b0..f6a24eb2e 100644 --- a/coreneuron/utils/profile/profiler_interface.h +++ b/coreneuron/utils/profile/profiler_interface.h @@ -15,7 +15,7 @@ #include #endif -#if defined(CORENEURON_CUDA_PROFILING) +#if defined(CORENEURON_CUDA_PROFILING) && (defined(__CUDACC__) || defined(_OPENACC)) #include #endif @@ -163,7 +163,7 @@ struct Caliper { #endif -#if defined(CORENEURON_CUDA_PROFILING) +#if defined(CORENEURON_CUDA_PROFILING) && (defined(__CUDACC__) || defined(_OPENACC)) struct CudaProfiling { inline static void phase_begin(const char* name){}; @@ -270,7 +270,7 @@ using InstrumentorImpl = detail::Instrumentor< #if defined CORENEURON_CALIPER detail::Caliper, #endif -#if defined(CORENEURON_CUDA_PROFILING) +#if defined(CORENEURON_CUDA_PROFILING) && (defined(__CUDACC__) || defined(_OPENACC)) detail::CudaProfiling, #endif #if defined(CRAYPAT) From 60b33ba220c80bddae4611d6bf851d9a1bbcfda1 Mon Sep 17 00:00:00 2001 From: Pramod Kumbhar Date: Mon, 18 Oct 2021 15:11:23 +0200 Subject: [PATCH 08/11] Remove extra variable CORENRN_ACC_FLAGS Linker should use -acc -cuda instead of just -cuda --- CMake/MakefileBuildOptions.cmake | 2 +- CMake/OpenAccHelper.cmake | 5 +---- coreneuron/CMakeLists.txt | 2 +- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/CMake/MakefileBuildOptions.cmake b/CMake/MakefileBuildOptions.cmake index c272aab1d..1e146c862 100644 --- a/CMake/MakefileBuildOptions.cmake +++ b/CMake/MakefileBuildOptions.cmake @@ -73,7 +73,7 @@ endforeach() string(REPLACE ";" " " CXX14_STD_FLAGS "${CMAKE_CXX14_STANDARD_COMPILE_OPTION}") string(TOUPPER "${CMAKE_BUILD_TYPE}" _BUILD_TYPE) set(CORENRN_CXX_FLAGS - "${CORENRN_ACC_FLAGS} ${CMAKE_CXX_FLAGS} ${CMAKE_CXX_FLAGS_${_BUILD_TYPE}} ${CXX14_STD_FLAGS} ${NVHPC_CXX_INLINE_FLAGS}" + "${NVHPC_ACC_COMP_FLAGS} ${CMAKE_CXX_FLAGS} ${CMAKE_CXX_FLAGS_${_BUILD_TYPE}} ${CXX14_STD_FLAGS} ${NVHPC_CXX_INLINE_FLAGS}" ) # ============================================================================= diff --git a/CMake/OpenAccHelper.cmake b/CMake/OpenAccHelper.cmake index ea55d9804..7767a3672 100644 --- a/CMake/OpenAccHelper.cmake +++ b/CMake/OpenAccHelper.cmake @@ -56,7 +56,7 @@ if(CORENRN_ENABLE_GPU) # more information about this. -gpu=cudaX.Y ensures that OpenACC code is compiled with the same # CUDA version as is used for the explicit CUDA code. set(NVHPC_ACC_COMP_FLAGS "-acc -gpu=cuda${CORENRN_CUDA_VERSION_SHORT}") - set(NVHPC_ACC_LINK_FLAGS "-cuda") + set(NVHPC_ACC_LINK_FLAGS "-acc -cuda") # Make sure that OpenACC code is generated for the same compute capabilities as the explicit CUDA # code. Otherwise there may be confusing linker errors. We cannot rely on nvcc and nvc++ using the # same default compute capabilities as each other, particularly on GPU-less build machines. @@ -65,9 +65,6 @@ if(CORENRN_ENABLE_GPU) endforeach() # avoid PGI adding standard compliant "-A" flags set(CMAKE_CXX14_STANDARD_COMPILE_OPTION --c++14) - set(CORENRN_ACC_FLAGS - "${NVHPC_ACC_COMP_FLAGS} ${PGI_DIAG_FLAGS}" - CACHE INTERNAL "") string(APPEND CMAKE_EXE_LINKER_FLAGS " ${NVHPC_ACC_LINK_FLAGS}") # Use `-Mautoinline` option to compile .cpp files generated from .mod files only. This is # especially needed when we compile with -O0 or -O1 optimisation level where we get link errors. diff --git a/coreneuron/CMakeLists.txt b/coreneuron/CMakeLists.txt index f07b900af..868053805 100644 --- a/coreneuron/CMakeLists.txt +++ b/coreneuron/CMakeLists.txt @@ -311,7 +311,7 @@ add_custom_target(nrniv-core ALL DEPENDS ${output_binaries}) include_directories(${CORENEURON_PROJECT_SOURCE_DIR}) if(CORENRN_ENABLE_GPU) - separate_arguments(CORENRN_ACC_FLAGS UNIX_COMMAND "${CORENRN_ACC_FLAGS}") + separate_arguments(CORENRN_ACC_FLAGS UNIX_COMMAND "${NVHPC_ACC_COMP_FLAGS}") target_compile_options(coreneuron PRIVATE $<$:${CORENRN_ACC_FLAGS}>) target_compile_options(scopmath PRIVATE $<$:${CORENRN_ACC_FLAGS}>) endif() From abd397ed9be86905881f274fa5f06b9c2811e8bf Mon Sep 17 00:00:00 2001 From: Pramod Kumbhar Date: Mon, 18 Oct 2021 15:37:25 +0200 Subject: [PATCH 09/11] Address code review comments --- CMake/MakefileBuildOptions.cmake | 2 +- coreneuron/CMakeLists.txt | 6 ++---- coreneuron/apps/corenrn_parameters.hpp | 2 +- coreneuron/apps/main1.cpp | 2 +- 4 files changed, 5 insertions(+), 7 deletions(-) diff --git a/CMake/MakefileBuildOptions.cmake b/CMake/MakefileBuildOptions.cmake index 1e146c862..fc0b0b551 100644 --- a/CMake/MakefileBuildOptions.cmake +++ b/CMake/MakefileBuildOptions.cmake @@ -73,7 +73,7 @@ endforeach() string(REPLACE ";" " " CXX14_STD_FLAGS "${CMAKE_CXX14_STANDARD_COMPILE_OPTION}") string(TOUPPER "${CMAKE_BUILD_TYPE}" _BUILD_TYPE) set(CORENRN_CXX_FLAGS - "${NVHPC_ACC_COMP_FLAGS} ${CMAKE_CXX_FLAGS} ${CMAKE_CXX_FLAGS_${_BUILD_TYPE}} ${CXX14_STD_FLAGS} ${NVHPC_CXX_INLINE_FLAGS}" + "${CMAKE_CXX_FLAGS} ${CMAKE_CXX_FLAGS_${_BUILD_TYPE}} ${CXX14_STD_FLAGS} ${NVHPC_ACC_COMP_FLAGS} ${NVHPC_CXX_INLINE_FLAGS}" ) # ============================================================================= diff --git a/coreneuron/CMakeLists.txt b/coreneuron/CMakeLists.txt index 868053805..6fd5c98a8 100644 --- a/coreneuron/CMakeLists.txt +++ b/coreneuron/CMakeLists.txt @@ -205,8 +205,6 @@ if(CORENRN_ENABLE_MPI AND CORENRN_ENABLE_MPI_DYNAMIC) # and should be aware of shared library suffix on different platforms. # ~~~ target_link_libraries(coreneuron ${CMAKE_DL_LIBS}) - target_compile_definitions(coreneuron - PUBLIC CORENRN_SHARED_LIBRARY_SUFFIX=${CMAKE_SHARED_LIBRARY_SUFFIX}) # store mpi library targets that will be built list(APPEND corenrn_mpi_targets "") @@ -312,8 +310,8 @@ include_directories(${CORENEURON_PROJECT_SOURCE_DIR}) if(CORENRN_ENABLE_GPU) separate_arguments(CORENRN_ACC_FLAGS UNIX_COMMAND "${NVHPC_ACC_COMP_FLAGS}") - target_compile_options(coreneuron PRIVATE $<$:${CORENRN_ACC_FLAGS}>) - target_compile_options(scopmath PRIVATE $<$:${CORENRN_ACC_FLAGS}>) + target_compile_options(coreneuron BEFORE PRIVATE $<$:${CORENRN_ACC_FLAGS}>) + target_compile_options(scopmath BEFORE PRIVATE $<$:${CORENRN_ACC_FLAGS}>) endif() # ============================================================================= diff --git a/coreneuron/apps/corenrn_parameters.hpp b/coreneuron/apps/corenrn_parameters.hpp index 5ad75eeeb..ea7ef8aba 100644 --- a/coreneuron/apps/corenrn_parameters.hpp +++ b/coreneuron/apps/corenrn_parameters.hpp @@ -52,7 +52,6 @@ struct corenrn_parameters { int seed = -1; /// Initialization seed for random number generator (int) bool mpi_enable = false; /// Enable MPI flag. - std::string mpi_lib; /// Name of CoreNEURON MPI library to load dynamically. bool skip_mpi_finalize = false; /// Skip MPI finalization bool multisend = false; /// Use Multisend spike exchange instead of Allgather. bool threading = false; /// Enable pthread/openmp @@ -86,6 +85,7 @@ struct corenrn_parameters { std::string reportfilepath; /// Reports configuration file. std::string checkpointpath; /// Enable checkpoint and specify directory to store related files. std::string writeParametersFilepath; /// Write parameters to this file + std::string mpi_lib; /// Name of CoreNEURON MPI library to load dynamically. CLI::App app{"CoreNeuron - Optimised Simulator Engine for NEURON."}; /// CLI app that performs /// CLI parsing diff --git a/coreneuron/apps/main1.cpp b/coreneuron/apps/main1.cpp index 6fba311c3..cdd618863 100644 --- a/coreneuron/apps/main1.cpp +++ b/coreneuron/apps/main1.cpp @@ -93,7 +93,7 @@ char* prepare_args(int& argc, char**& argv, int use_mpi, const char* mpi_lib, co } // if neuron has passed name of MPI library then add it to CLI - std::string corenrn_mpi_lib(mpi_lib); + std::string corenrn_mpi_lib{mpi_lib}; if (!corenrn_mpi_lib.empty()) { args.append(" --mpi-lib "); corenrn_mpi_lib += " "; From 5afe8a294926c045c550c135cc3d89a38581b780 Mon Sep 17 00:00:00 2001 From: Pramod S Kumbhar Date: Wed, 20 Oct 2021 20:17:47 +0200 Subject: [PATCH 10/11] Update mod2c to latest master --- external/mod2c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/external/mod2c b/external/mod2c index 2234d281b..bcf471291 160000 --- a/external/mod2c +++ b/external/mod2c @@ -1 +1 @@ -Subproject commit 2234d281baa2528e19afe59de441e638b6b0bb8c +Subproject commit bcf471291ed615e3d2df7e06f0a27aa38888cff4 From 5106f4094c7080c0cfdb26b30010bbd412aea6b2 Mon Sep 17 00:00:00 2001 From: Pramod S Kumbhar Date: Wed, 20 Oct 2021 23:38:32 +0200 Subject: [PATCH 11/11] Address review comments --- coreneuron/apps/main1.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/coreneuron/apps/main1.cpp b/coreneuron/apps/main1.cpp index cdd618863..ff664a35f 100644 --- a/coreneuron/apps/main1.cpp +++ b/coreneuron/apps/main1.cpp @@ -460,8 +460,6 @@ std::unique_ptr create_report_handler(ReportConfiguration& config using namespace coreneuron; #if NRNMPI -#define STRINGIFY(x) #x -#define TOSTRING(x) STRINGIFY(x) static void* load_dynamic_mpi(const std::string& libname) { dlerror(); void* handle = dlopen(libname.c_str(), RTLD_NOW | RTLD_GLOBAL);