-
Notifications
You must be signed in to change notification settings - Fork 122
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.
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
Support for CMake based build system, tests, documentation and CI #341
Conversation
The goal of this work is to support cmake build system for neuron. We will keep compatibility with existing autoconf build system as well. * Beginning cmake configuration WIP. * Add executable for nrniv * cmake working nrniv executable with built-in mod files * Use the external iv library using cmake find_package (#242) We use `find_package(iv)` now to populate cmake targets coming from interViews in this project. As defined in interViews cmake files, all these targets are prefixed with `iv_` to prevent overwriting. So, if we would like to link against the `interviews` target we must use here `iv_interviews`. Since in the interViews cmake files we also define the interface headers, they are correctly picked up here when compiling objects that will be ultimately linked against `iv_interviews`. To help `find_package` to do its job, we should start cmake as follows: ``` CMAKE_PREFIX_PATH=/path/to/iv/installation/share/cmake/iv/ cmake .. ``` * nrniv builds with CMAKE_PREFIX_PATH=/home/hines/neuron/ivcmake/build/install/share/cmake/iv cmake .. -DCMAKE_INSTALL_PREFIX=install * InterViews GUI and mpi works. But need cp -r ../aconf/share/lib/* install/share/nrn/lib for nrn.defaults * Turn on cmake configure_file. Most identical to autoconf subsitution. * Cmake install nrngui, neurondemo, nrnivmodl But nrnmech_makefile is too wedded to autoconf (libtool) to work in a cmake context. * neurondemo works with cmake nrnivmodl still basically the same but steers to nrnmech_makefile derived from nrnivmodl_makefile.cmk * CMake changes to build on OS X (#244) * Fixes for readline and osx builds: (#245) - dynamic_lookup flags for osx is only portable with clang - add missing mpi includes - use internal readline on OSX or iff readline not found - add Findreadline.cmake * MAC needs an -isysroot for compiling in nrnmech_makefile Co-authored-by: Pramod Kumbhar <[email protected]> Co-authored-by: Michael Hines <[email protected]>
* Minimum cmake config for python module built by setup.py * The cmake options NRNPYTHON_DYNAMIC and INSTALL_NEURON_MODULE work. - Default python3 falling back to python. - If NRNPYTHON_DYNAMIC is in effect the NRNPYTHON_SRC is no longer in libnrniv and libnrnpython2.so or libnrnpython3.so are loaded as needed depending on the python version that is being used. * If INSTALL_NEURON_MODULE is YES - Setup.py for each python executable in the NRNPYTHON_DYNAMIC list (or the default python executable if NRNPYTHON_DYNAMIC is YES or NO) is used to install the neuron modules using --home=${CMAKE_INSTALL_PREFIX} . If the value of INSTALL_NEURON_MODULE is neither YES nor NO, the module is installed with <python> setup.py install <value of INSTALL_NEURON_MODULE> * All the ; separated pythons work and set a valid include path that contains Python.h * With cmake the old issue of Python launch on LINUX not resolving variables from already loaded shared libraries has returned. Work around is to promote libnrniv.so to RTLD_NOW | RTLD_NOLOAD | RTLD_GLOBAL
* Cmake -DENABLE_INTERVIEWS=OFF works * CMake build system refactoring (#247) - Top level refactoring 5 - Add build status after cmake stage - CMake modules refactoring - Avoid python2 library detection when default python is 3 - Python | 1 - EXE | /usr/local/Cellar/python/3.7.2_2/bin/python - LIB | /usr/lib/libpython2.7.dylib * Prefer version detected as interpreter * Bug fix for python library detection, use pybind11 module which has worked very well. - See: pybind/pybind11#207 * Rpath issue and nrnivmodl fixes (#249) * Remove interview target link while building special * Fix rpath issue with shared libraries - see https://stackoverflow.com/questions/32283062/compiling-c-shared-library-with-distutils-setup-py-when-the-library-depends-on - include IV directories * Fix link error with dynamic mpi * Fix missing darwin definition: (#253) This was causing following error: testNumpyInteraction (neuron.tests.test_vector.VectorTestCase) Testing numpy.array <=> hoc.Vector interaction ... build_cmake.sh: line 16: 11620 Segmentation fault: 11 $python -c 'from neuron import test; test()' * Set missing IV_LIBDIR (#254) * Add missing rpath for interviews lib directory * Small changes while testing on Ubuntu - print python include directory (which could be missing) - check python dir exist to avoid comple time error * Fix missing linking with -lpthread and -ldl with dynamic mpi and python on linux * Fix issue with python dynamic support is enabled - I see below error when trying to use python dynamic build: ``` nrn/install-dir/x86_64/bin/nrniv -python -pyexe python3.7 -c 'import neuron ; neuron.test() ;' NEURON -- VERSION 7.7.1-24-g4524ea3c master (4524ea3) 2019-07-31 Duke, Yale, and the BlueBrain Project -- Copyright 1984-2018 See http://neuron.yale.edu/neuron/credits bash: nrnpyenv.sh: No such file or directory Python not available ``` - This is because popen fails for `bash nrnpyenv.sh` fails. * Install nrnpyenv.sh to install-prefix/bin Co-authored-by: Pramod Kumbhar <[email protected]> Co-authored-by: Michael Hines <[email protected]>
* bugfix: rxd 3D initialization was missing some segments. (#246) * Update deprecated MPI_Adress and MPI_Type_struct. Fix if(a=b) warning. * NMODL for non-vectorized models did not set _p and _ppvar for NET_RECEIVE INITIAL * NETRECEIVE INITIAL blocks called in NetCon creation order. * Update hints on how to build for mingw. * nocmodl buf for INCLUDE file introduced by Nmodltext changeset 09f4512 But also, there should be no limit on size of MODL_INCLUDE which is a ':' separated list of paths. * sorted region._sa by points (#250) * bugfix: bugfix: added call to nrn_shape_update so "h.doNofify()" call triggers an update to the number of rxd nodes when nseg is changed. (#252)
* I see below error when trying to use python dynamic build: ``` nrn/install-dir/x86_64/bin/nrniv -python -pyexe python3.7 -c 'import neuron ; neuron.test() ;' NEURON -- VERSION 7.7.1-24-g4524ea3c master (4524ea3) 2019-07-31 Duke, Yale, and the BlueBrain Project -- Copyright 1984-2018 See http://neuron.yale.edu/neuron/credits bash: nrnpyenv.sh: No such file or directory Python not available ``` - This is because popen fails for `bash nrnpyenv.sh` fails. * Install modlunit, mkthreadsafe, and sortspike.
* (WIP) Fixes for windows build with CMake It now builds till nrniv.so: kumbhar@DESKTOP-A627701 MSYS ~/nrn/build $ make [ 6%] Built target nocmodl [ 9%] Built target modlunit Scanning dependencies of target nrniv_lib [ 9%] Building C object src/nrniv/CMakeFiles/nrniv_lib.dir/__/oc/cygwinprt.c.obj [ 10%] Linking CXX shared library libnrniv.so CMakeFiles/nrniv_lib.dir/__/nrnoc/init.c.obj: In function `hoc_last_init': C:/msys64/home/kumbhar/nrn/src/nrnoc/init.c:351: undefined reference to `modl_reg' collect2.exe: error: ld returned 1 exit status make[2]: *** [src/nrniv/CMakeFiles/nrniv_lib.dir/build.make:6422: src/nrniv/libnrniv.so] Error 1 make[1]: *** [CMakeFiles/Makefile2:181: src/nrniv/CMakeFiles/nrniv_lib.dir/all] Error 2 make: *** [Makefile:130: all] Error 2 * Add missing termcap module - find termcap before adding nrniv folder - do not link to libdl on mingw platform * Various to make NEURON buildable under windows: - make cmake option consistent for shared library - replace find command with CMake's find(GLOB_RECURSE.. so that it becomes windows compatiable (find.exe has different options than *nix) - link stnadard libc and libc++ in static mode for mingw/windows build - use setup.cfg to tell distutils to use mingw toolchain instead of visual studio - setup.py is updated to translate C:/msys64 to C:\\msys64 so that windows python understand paths correctly - adjust python install --home argument for windows * More cmake cleanup (#260) * Added missing dependency rules * Fixed outstanding issue with cmake config files * Moved nrnivmodl makefile generator to right dir. * Windows InterViews does not need X11 and backslash needs escape. * With MINGW -DNRN_ENABLE_INTERVIEWS kind of works. * CMAKE_INSTALL_BINDIR and others did not get set. * Cmake MATCHES to STREQUAL as "" matches anything. * Only add python interface to libnrniv if not dynamic and python enabled * WIP for NRN_ENABLE_PYTHON_DYNAMIC for MINGW MINGW libnrnpython needs to link against libpython to avoid undeclared names. So introduce libnrnpython<major><minor> library targets for each python in the NRN_PYTHON_DYNAMIC list if LINK_AGAINST_PYTHON is ON (now hardwired as ON in PythonDynamicHelper.cmake for development on MAC but only needed for MINGW). Currently what is missing is for nrnpy.cpp to figure out <major><minor> suffix when nrniv is launched. * Proper permissions for installed scripts. * When launch nrniv, calculate libnrnpython name from python library. * MINGW python dynamic loading fixes * USE_LIBNRNPYTHON_MAJORMINOR needs to be 1 or 0 for #define * Cmake build report enhancement and cleanup (#274) * CMake cleanup with build report * Add NEURON_INIT_MPI possibilities when dynamic loading MPI (#261) - If NEURON_INIT_MPI==0 dont load MPI library; else load MPI * mkdir with MINGW takes only one arg. Co-authored-by: Pramod Kumbhar <[email protected]> Co-authored-by: Michael Hines <[email protected]>
* Use PROJECT_SOURCE_DIR for submodules * Change travis config from trusty to bionic and explicit about python version (#279) * Allow Interviews to be submodule of NEURON Now one can use neuron as: cmake .. -DNRN_ENABLE_INTERVIEWS=OFF # no interviews cmake .. -DNRN_ENABLE_INTERVIEWS=ON -DCMAKE_INSTALL_PREFIX=`pwd`/install #build Interviews from submodule cmake .. -DNRN_ENABLE_INTERVIEWS=ON -DCMAKE_INSTALL_PREFIX=`pwd`/install -Div_DIR=$HOME/iv/install/share/cmake/iv #use externally installed interviews * Update iv submodule and change remote url Change iv_interviews to interviews; fix search path (#286) * Added coreneuron as submodule (WIP) - change cmd options : iv_DIR to IV_DIR and coreneuron_DIR to CORENEURON_DIR - install nrnivmodl-core to NEURON install-prefix/bin - update submodules - Look for coreneuron library in <arch_dir>/.libs/libcorenrnmech.so prior to CORENEURONLIB env variable * Update coreneuron with fix for mod2c binary
* cmake -DNRN_ENABLE_RX3D=ON works on mac. * To save a lot of time, the rx3d modules are compiled with -O2. * Python module rebuilt if setup.py.in changes or the file dependency touched. For now, dependencies that copy inithoc.cpp have to be explicitly mentioned. * Only create cpp from pyx if python enabled and module install enabled * Begin 7.8 development (#292) * Update NEURON version for cmake * ctypes in neuron/__init__.py with cmake on windows problems. * Fixed for MINGW by installing dlls in <inst>/bin and setting NEURONHOME to <inst> and installing a bash.exe in <inst>/mingw/usr/bin * rx3d building and working with cmake * -DNRN_RX3D_OPT_LEVEL=2 determines optimizaton level of cython generated files.
* mingw shell script did not work for mac. Simplified the script for mac and use cmake logic to determine which script to use. * autotools and cmake work in the same branch * Must not expand a possibly nonexistent variable. * Custom target for cython generated files much simplified by use of MS_WIN64 define for mingw. * Build the cython related modules on windows using msvc. Follows the autotools strategy in share/lib/python/neuron/rxd/geometry3d/Makefile.am
* digittoint not standard on linux * Use neuron.test() instead of rxd specific test. * update submodules : iv as well as coreneuron (#302) * Array for spike compression may not be allocated in some circumstances. (#293) * Remove legacy options for purify and mpitrace library of bg-q (#296) * Avoid bash warning/error for empty if block (#298) * add back the run_all.py travis_cl test * Reviewed all assert for necessary side effects. (#306) (#307) 30 converted from assert to nrn_assert. So work properly with NDEBUG defined. Co-authored-by: Pramod Kumbhar <[email protected]> Co-authored-by: Michael Hines <[email protected]>
* travis CI: add test for cmake build * explicitly turn on/off interviews * add python tests for cmake build * Fix rxd tests with CMake - use sys.executable to get path of python used to launch tests (this is because on my machine there was no python2 but python2.7. So using sys.executable seems more robust?) - build librxdmath - run rxd tests on travis * update submodules to the latest version; remove unnecessary comment in MacroHelper * Fix travis.yml to correctly detect failures (#314) * Add build for mac osx Co-authored-by: Pramod Kumbhar <[email protected]> Co-authored-by: Omar Awile <[email protected]>
- Newly added cmake code / files are now formatted using cmake-format package - coding-convention submodule is added but there is still some work needed to avoid formatting of comment blocks (issue opened in BBP repository) - Add submodule and initialize it if formatting option is added - Disable recusive clonning in travis - use cmake format fence ~~~ to avoid comment block formatting - only modules directory not formated properly otherwise one can do cmake .. -DNEURON_CMAKE_FORMAT=ON make cmake-format git checkout ../cmake/modules/ # undo formatting of cmake modules
* An initial commit to get code documentation into NEURON * Added cmake target and generate docs inside build dir * Added some cmake instructions into README.md * Added building doxygen in travis and deploying docs * fixed travis script to build doc for one job * Small fix in README.md cmake options * Added cmake option NRN_BUILD_DOC and selectively build doc in travis
* Introduce failure to validate travis CI * add dependencies of nrnpython<ver> to hoc_module * Fixed all tests * Fix travis.yml to correctly detect failures * Avoid setting rpath of build library : it was causing quite confusion and failures from dlopen. We will use LD_LIBRARY_PATH or DYLD_LIBRARY_PATH mechanism. From build directory we will use: NEURONHOME=`pwd`/share/nrn/ \ PYTHONPATH=`pwd`/lib/python \ python3.6 -c "from neuron import h; import neuron; neuron.test()" * Avoid rpath to build but add it to libdir for linking * Added simple catch2 test * Added small hoc test * Added pytest framework and a basic test * Added hoc interpreter test in catch2 and still working on fast_imem test * Fixed fast_imem test * Simple python test * Fix to clear fast_imem * Refactored fast_imem test with scenario and enriched the test * Fixed CMakeLists.txt to set the proper environment (and fixed a forgotten diff in the python tests) * Updated travis to run make test * Enable tests in cmake on travis * new coreneuron commit : don't run tests if coreneuron is submodule * Added HOC ringtest * Set environment for the hoc ringtest * Fixes for unit test and added rxd pytest stubs * Added spikes pytest and reformated catch tests * Added seperate catch test * Aded first rxd python tests with pytest (wip) * bugfix: improved rxd cleanup so the initial rxd pytests work. (#330) * Fixed Species destructor - Created unit_tests, hoc_tests and python_tests folders for catch, hoc and python tests respectively - Added RunHOCTest.cmake which launches nrniv and then validates the results of the tests - HOC tests (ringtest and connect_dend) need output validation because nrniv doesn't output an error to bash if it fails - Fixed hoc tests' environment - Deleted comments from unit_test.cpp * copy hoc files into build dir for tests. Might repeate the same op if python and tests are enabled but ensures that all is there if either is enabled * set PYTHONPATH only if python is enabled * Add another reference dat file for coreneuron enabled builds (which have a patched hh.mod) * Added sdiff to check differences in results in CI and removed some code copying in CMakeLists of test * Removed catch_discover_tests since it didn't work properly - Environment variables were not set to run the catch test executable to discover the tests it includes * Added a new argument to ivocmain that makes dropping into session optional The `start_session` argument is set to 1 if the Oc.session shoudl be run, which drops into the hoc interpreter interactive session. If this parameter is passed as 0, then we return without starting the session. * Small improvement to non-interactive calling of ivocmain and build fix * Undo change to ivocmain and introduce new function to preserve API * Also added doxygen to document this function. Co-authored-by: Ioannis Magkanaris <[email protected]> Co-authored-by: adam newton <[email protected]> Co-authored-by: Michael Hines <[email protected]> Co-authored-by: Pramod Kumbhar <[email protected]>
…ON (#337) if NEURON was first build with NRN_ENABLE_CORENEURON=ON and then CoreNEURON was disabled and the project was rebuilt
- headers and other files are now placed in consistent relative paths in the build path compared to the install path. This allows us to run nrnivmodl both in build and install with consistent results. - nrnivmodl_makefile now uses an additional `ROOT` variable as its prefix path, rather than hardcoding the prefix at compile time. - nrnivmodl uses more consistent paths and passes the ROOT path to the Makefile
Co-Authored-By: Omar Awile <[email protected]>
Additionally modified, removing NEURONHOME becuase it prevents rxd from finding librxdmath and adding $ENV{SHELL} becuase nrnivmodl is installed without execute permissions.
- when cmake branch was squashred and rebased, certain changes were lost due (mostly human errors!) - copied changes from upstream cmake by comparisong against rebased branch
Used as: - cmake . -DNEURON_CMAKE_FORMAT=ON - make cmake-format && git checkout cmake/modules
@wvangeit: As @nrnhines has verified whole ModelDB testsuite with this branch, if time permits, it will be great if you could run your CI. Don’t need to use cmake build system or anything. Just use this branch with regular autoconf build options (and we expect everything to be same as master branch). |
I successfully ran the modeldb and nrntest tests with the option-defaults branch which branched off cmake_master_merge at 4399e0c |
@DrTaDa and @anilbey, can you run the BluePyOpt and BGLibPy tests on the Neuron branch related to this PR (pramodk/cmake_master_merge) For BGLibPy, make sure to also use the v5 an v6 test targets (on BB5). |
All tests passed for Output:
|
@wvangeit All tests passed for BluePyOpt as well ! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Out of context, your original change makes sense. When, in fact, does this get freed?
b6e3a2e
to
de49d9f
Compare
* Fixed rxd test rules, module building and updated nrnivmodl.in * Bump hpc-coding-conventions git submodule to v1.0.0 * add .gitignore / format CMake files / better error messages * Added doxygen brew package in osx to build docs in osx as well but deploy only form linux * Small fix in copying hh.mod and removing hh.mod.orig file thanks to @ohm314 * Add unit tests for CoreNEURON direct memory transfer mode * Cleanup CMake logic for patching hh.mod * Various fixes thanks to @tristan0x and @ferdonline's reviews * CMake emits an error when pytest is missing but required
* Apply flake8 and black on test/pynrn and test/rxd Python files * Added docstrings for rxd tests. * Fix rxd tests: keep reference to native objects / fix formatting * Add missing test/coreneuron/test_direct.py * More minor fixes for tests in the cmake files
similarly to nrnmech_makefile.in
* Bumped CoreNeuron to latest commit from master * Update cmake build type configuration in coreneuron and iv * compiler flags were getting overwritten by the sub modules, resulting in builds with -O0 that were supposed to be with -O2 (Debug vs. RelWithDebInfo). * Update iv to latest master
…tic for computing and reporting membrane current
de49d9f
to
61217d3
Compare
@nrnhines : This is finally ready for merge to master! Fernando and Omar has squashed some commits yesterday to reduce total number of commits to be merged with master. As mentioned in the Todo list, we have done all cross-checking. @iomaganaris @ferdonline @ohm314 : can you approve this PR? @nrnhines : if there any questions, you can discuss with @ohm314 in todays call and then we can merge this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you all!
Phew..! Huge thanks to everyone : @nrnhines @ramcdougal @adamjhn @ferdonline @ohm314 @iomaganaris @WeinaJi @salvadord @jamesgking @wvangeit @DrTaDa @anilbey @tristan0x @wwlytton ! |
The goal of this PR is to integrate efforts of multiple teams to bring:
Note : I have squashed commits and prepare relatively small number of commits that we should keep. So we should create a merge commit from this PR.
How to build this branch?
bash build.sh ./configure --prefix=`pwd`/install --with-nrnpython --with-paranrn --without-x make -j make install
Once you have CMake installed on your system (see here extra packages for other build options):
See travis.yml for details.
Todo:
- [ ] Testing of netpyne against this branch : @salvadord : could you run netpyne tests against this branch?Thanks to significant efforts from multiple teams during codejam : @nrnhines @ramcdougal @adamjhn @ferdonline @ohm314 @iomaganaris @WeinaJi @salvadord @jamesgking