Improve nrnivmodl-core workflow and integration with nmodl/mod2c #388

pramodk · 2020-08-31T19:18:33Z

Main motivation of this PR is to streamline workflow so that the MOD files can be compiled for different backends or using different MOD2C/NRNIVMODL flags.
Currently inbuilt mechanisms are compiled during CoreNEURON build time. If user want to compile mod files with different NMODL flags, it's not possible to do it without re-compiling CoreNEURON itself.
In this PR we removed compilation of mod files from CMake as part of libcoreneuron.
Mod files are now compiled with Makefile + nrnivmodl-core script and resultant library is linked to special-core or nrniv-core.

Build all mod files uniformly by removing NmodlHelper.cmake
everyone now use nrnivmodl-core to build binary
internal mod files are now also compiled via nrnivmdl-core
ispc target also use nrnivmodl-core workflow
Remove extra library libcorenrnmech built
Remove dest option in nrnivmld-core
Remove nrnivmodl_options.txt SAVE file
Avoid installation of libcorenrnmech into bin directory
Cleanup and simplify nrnivmodl-core.in and nrnivmodl_core_makefile.in

pramodk · 2020-09-07T21:58:23Z

@ferdonline @alexsavulescu @ohm314 @iomaganaris : I don't know Makefile well but I have significantly changed implementation to make scripts & makefie bit easy to understand. Take a quick look and we can see what needs to be changed.

extra/nrnivmodl-core.in

ferdonline · 2020-09-08T16:16:36Z

extra/nrnivmodl-core.in


+# copy mod files with include files. note that ${ROOT_DIR}/share


The idea is we prepare the mod files here so that the makefile can be simpler?

Btw, for reference, maybe we can update https://user-images.githubusercontent.com/915100/53173802-ad701780-35e8-11e9-9d28-0441aecbf517.png (in #133 (comment))

@ferdonline : main motivation for recompiling all mod files is to have flexibility in building special. For example, once CoreNeuron is installed, we want to compile mod files with mod2c or nmodl or different nmodl options or different compiler flags.

If inbuilt mod files are built inside libcoreneuron, one has to completely re-install CoreNeuron with different flags of nmodl. This is quite restrictive for new usage with NMODL. And hence, we no longer want to compile mod files inside libcoreneuron.

Btw, for reference, maybe we can update https://user-images.githubusercontent.com/915100/53173802-ad701780-35e8-11e9-9d28-0441aecbf517.png (in #133 (comment))

Could you provide the source of that? :)

ferdonline · 2020-09-08T16:23:59Z

Really cool to have dropped this old mode of building mods in the main Cmake and removed NmodlHelper.cmake.
As an idea maybe we can move some logic from the makefile to the bash script. bash is not pretty (maybe we could think about Python ?) but certainly makefile functions are harder to maintain. I believe keeping the makefile to the bare role of building while taking care of dependencies would be good.

ohm314

Really nice. Love the cleanup of the makefile! I just have a few questions/minor comments.

CMakeLists.txt

CMake/NmodlHelper.cmake

extra/nrnivmodl-core.in

extra/nrnivmodl_core_makefile.in

iomaganaris

LGTM overall. I don't have much to add on the comments of the others

extra/nrnivmodl-core.in

pramodk · 2020-09-15T22:20:40Z

As an idea maybe we can move some logic from the makefile to the bash script. bash is not pretty (maybe we could think about Python ?) but certainly makefile functions are harder to maintain.

@ferdonline : I agree that this could be further simplified and cleaned. I am thinking that we should make such change in separate PR so that we don't change too many things in the same PR (especially introducing new things with bash or python). With the current changes, I am more or less confident that it won't break too many things :)

pramodk · 2020-09-16T06:37:14Z

Please retest

- remove destination option - avoid re-compilation of mod files - minor cleanup of makefile - remove unused variables and install target

- install special-core as nrniv-core - install libcorenrnmech into bin directory - rmeove unused cmake code

pramodk · 2020-09-21T15:37:26Z

Please retest

ohm314

looks good!

…eBrain/CoreNeuron#388) * nrnivmodl-core cleanup - remove destination option - avoid re-compilation of mod files - minor cleanup of makefile - remove unused variables and install target * Remove unused CMake module and code for new workflow - remove libnrnmech which is not used (?) - install special-core as nrniv-core - install libcorenrnmech into bin directory - rmeove unused cmake code * install mod files from /share * fix test links * Add dependency between coreneuron_test for mech library * linking fixes for GPU build * Small changes/refactoring in nrnivmodl-core.in * Refactoring of target rules * Cleanup makefile rules more * Add dependency with nmodl target * Change INSTALL_ to CORENRN_ prefix and address review comment * Build libcornrnmech static one always required for nrniv-core * Add CLI option to choose static or shared build CoreNEURON Repo SHA: BlueBrain/CoreNeuron@cc85723

pramodk force-pushed the pramodk/nrnivmodl-uniform-workflow branch from 65fd5bd to bdd1aaf Compare September 4, 2020 08:45

pramodk requested review from ferdonline, ohm314, iomaganaris and alexsavulescu and removed request for iomaganaris September 7, 2020 21:53

ferdonline reviewed Sep 8, 2020

View reviewed changes

extra/nrnivmodl-core.in Show resolved Hide resolved

ferdonline reviewed Sep 8, 2020

View reviewed changes

ohm314 reviewed Sep 11, 2020

View reviewed changes

CMakeLists.txt Show resolved Hide resolved

CMake/NmodlHelper.cmake Show resolved Hide resolved

extra/nrnivmodl-core.in Show resolved Hide resolved

extra/nrnivmodl_core_makefile.in Outdated Show resolved Hide resolved

extra/nrnivmodl_core_makefile.in Outdated Show resolved Hide resolved

iomaganaris reviewed Sep 15, 2020

View reviewed changes

extra/nrnivmodl-core.in Show resolved Hide resolved

extra/nrnivmodl-core.in Outdated Show resolved Hide resolved

pramodk added 16 commits September 21, 2020 13:27

WIP : make now builds special-core for tests

3237bd4

nrnivmodl-core cleanup

20c3714

- remove destination option - avoid re-compilation of mod files - minor cleanup of makefile - remove unused variables and install target

Remove unused CMake module and code for new workflow

f9fda3c

- remove libnrnmech which is not used (?)

d0b6e8c

- install special-core as nrniv-core - install libcorenrnmech into bin directory - rmeove unused cmake code

apply cmake-format

a1afb6a

install mod files from /share

14bdb07

fix test links

add49a7

Add dependency between coreneuron_test for mech library

ecaebff

linking fixes for GPU build

bc50840

Small changes/refactoring in nrnivmodl-core.in

fbcb09f

intermediate commit

d73a646

Refactoring of target rules

7637e14

Cleanup makefile rules more

23cd2df

refactoring more targets

abb1b0b

more makefile refactor

2277111

fixed non-ispc targets

eae303d

pramodk added 17 commits September 21, 2020 13:27

fix missing single quotes for rpath

a6540f5

symlink nrnunits

6ed9a26

travis verbose

f3b7081

fix link error in static builds

1fc9b8e

Set visiblity to hidden to avoid double free

d82a8c8

undo visibility hidden; instead avoid linking licoreneuron

0ac5343

Set rpath for build time exes

6763bd3

Add coreneuron library linking

d0b9961

libcoreneuron is built with SHARED and vice-a-versa

2678cc2

Allow undefined reference in shared library

1ced129

limit to two parallel builds with make

5208532

Add dependency with nmodl target

e461e0c

More cleanup and documentation; serial build

ab3c381

rename target used by neuron : coreneuron_test to nrniv-core

c8501c9

update tests with the target

a4d36dc

Change INSTALL_ to CORENRN_ prefix and address review comment

250eb92

Add install target and re-introduce -d option by popular demand

8dfa544

pramodk force-pushed the pramodk/nrnivmodl-uniform-workflow branch from 1c87231 to 8dfa544 Compare September 21, 2020 11:27

pramodk marked this pull request as ready for review September 21, 2020 11:28

pramodk added 2 commits September 21, 2020 16:18

Build libcornrnmech static one always required for nrniv-core

7e9fa20

Add CLI option to choose static or shared build

1035a0d

iomaganaris approved these changes Sep 21, 2020

View reviewed changes

ohm314 approved these changes Sep 21, 2020

View reviewed changes

pramodk merged commit cc85723 into master Sep 21, 2020

pramodk deleted the pramodk/nrnivmodl-uniform-workflow branch September 21, 2020 17:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve nrnivmodl-core workflow and integration with nmodl/mod2c #388

Improve nrnivmodl-core workflow and integration with nmodl/mod2c #388

pramodk commented Aug 31, 2020 •

edited

Loading

pramodk commented Sep 7, 2020

ferdonline Sep 8, 2020 •

edited

Loading

pramodk Sep 15, 2020

pramodk Sep 15, 2020

ferdonline commented Sep 8, 2020

ohm314 left a comment

iomaganaris left a comment

pramodk commented Sep 15, 2020

pramodk commented Sep 16, 2020

pramodk commented Sep 21, 2020

ohm314 left a comment


		# copy mod files with include files. note that ${ROOT_DIR}/share

Improve nrnivmodl-core workflow and integration with nmodl/mod2c #388

Improve nrnivmodl-core workflow and integration with nmodl/mod2c #388

Conversation

pramodk commented Aug 31, 2020 • edited Loading

pramodk commented Sep 7, 2020

ferdonline Sep 8, 2020 • edited Loading

Choose a reason for hiding this comment

pramodk Sep 15, 2020

Choose a reason for hiding this comment

pramodk Sep 15, 2020

Choose a reason for hiding this comment

ferdonline commented Sep 8, 2020

ohm314 left a comment

Choose a reason for hiding this comment

iomaganaris left a comment

Choose a reason for hiding this comment

pramodk commented Sep 15, 2020

pramodk commented Sep 16, 2020

pramodk commented Sep 21, 2020

ohm314 left a comment

Choose a reason for hiding this comment

pramodk commented Aug 31, 2020 •

edited

Loading

ferdonline Sep 8, 2020 •

edited

Loading