Fixing nrnivmodl-core: Avoid redefined symbols + script startup fixes #133

ferdonline · 2019-02-18T15:50:40Z

Fixes to nrnivmodl-core and required changes in the build of coreneuron

Current implementation of nrnivmodl-core suffers from a complex problem, redefinition of some symbols which depend on the mechanism set: modl_reg() and those defined in dimplic.cpp

This PR fixes that problem (and other little ones) from the root which required changes in the organization of coreneuron libraries and executables.

Short summary:

libcoreneuron shall be a "kernel" lib, i.e. it shall not contain extra mechanisms and their dependencies, but "complete", i.e.: It contains the standard mechanisms and everything (no undefined symbols) to be able to run on standard simulations.
- modl_reg() call was accordingly moved out main1.cpp
- scopmath_core package excluded from the libcoreneuron too
libcoreneuron's solve_core was split into mk_mech_init and run_solve_core
A new (coreneuron+mechanisms) lib exists (corenrnmech) for the previous purpose, which includes
- mechanisms per se, modl_reg(). solve_core() (which calls modl_reg()) and full scopmath_core (+ dimplic.cpp)
- links against libcoreneuron
coreneuron main therefore only has to do a bare solve_core() from corenrnmech lib (importing the new enginemech.h header)

When building mods with nrnivmodl-core the process is the same!
Note that in order to optionally build-up on previous mechanism, the ADDITIONAL mechamisms are exported as cpp files to share/mod2c and bundled together with the new mods cpp's.
See the diagram below.

ferdonline · 2019-02-21T13:56:52Z

Updated nrnivmodl-core diagram:

ferdonline · 2019-02-21T16:54:56Z

@pramodk I don't think we expected this much work :p but it was a nice challenge!
We might need to adapt some software though in case they link to the previous libcoreneuron and expect to have the extra machanisms included

ferdonline · 2019-02-22T09:27:29Z

extra/CMakeLists.txt

+# Disable new-dtags with newer linkers
+############################################################
+
+if (UNIX AND NOT APPLE)


If you want to avoid this workaround I think we could live with RUNPATH by setting it also in the shared libraries (apparently it allows that). Shall I experiment with that?

without that --disable-dtags option, the RUNPATH is populated correctly but still get library not found error. Didnt understand that quite well.

It might be related to the fact that they are used a bit differently by the dynamic loader: See Bug #1253638 “dynamic linker does not use DT_RUNPATH for transitive dependencies
If I understood well, RUNPATH is only valid for direct dependencies, but sub-dependencies can also define RUNPATH which gets also checked. I kind of agree with the new behavior, but true, breaks things

pramodk · 2019-02-22T09:52:25Z

@pramodk I don't think we expected this much work :p but it was a nice challenge!

yeah....too much headache though! :)

ferdonline · 2019-02-22T20:26:17Z

@pramod I linked using gold, checked it produces RUNPATH, but still it all works in bb5. Is it something specific to ubuntu?
I noticed that libcoreneuron is directly linked so it should indeed work.
To make the test, in the last commit I added rpath also to the lib (it seems RUNPATH supports that in libs) and removed your patch. Check if that also works with you, otherwise please discard that commit.

ferdonline · 2019-02-26T19:23:17Z

coreneuron/CMakeLists.txt

@@ -315,8 +312,8 @@ add_library(coreneuron ${COMPILE_LIBRARY_TYPE} ${coreneuron_all_headers} ${coren
                       ${coreneuron_all_c_files} ${cudacorenrn_objs} ${MOD2C_STDMECH_OUTPUTS})
 add_dependencies(coreneuron kinderiv)

-target_link_libraries(coreneuron ${MPI_CXX_LIBRARIES} 
-                      ${link_reportinglib} ${CUDA_LIBRARIES})
+target_link_libraries(coreneuron ${MPI_CXX_LIBRARIES}


${cudacorenrn_objs} doesnt exist now, right?

No longer first-found functions in linking. We now strip scoptmath from the core and link together with generated mods when creating coreneuron_app or special-core

To avoid client applications to use three instructions every time they need to run core neuron with mechanisms, we create an intermediate library (corenrnmech, similar to the process of nrnivmodl-core) which contains the extra mechanisms and a solve_core() launcher, besides all dependent registration/hook routines (mod_func.cpp and dimplic.cpp)

Some newer systems use the gold linker which creates RUNPATH instead of RPATH. This may be problematic as apparently it doesnt propagate the RPATH to dependencies. To mitigate the problem (without disabling new dtags) we: - Set libcoreneuron as a direct dependency - Set rpath on libcorenrnmech itself (LD should ignore, GOLD should take it) - This shouldnt be needed due to the previous measure, but it's actually a good practice.

In mac $ORIGIN is called @loader_path. Therefore mac builds would only work once installed. Fixed. Second. To avoid having a superfluous RPATH, now we create a nrnivmech_install.sh script which calls gnu make "install". It will relink the executable setting the destination RPATH, and install the files

ferdonline · 2019-02-26T22:12:44Z

coreneuron/CMakeLists.txt

+# The coreneuron lib (only internal mechanisms)
+#
+add_library(coreneuron ${COMPILE_LIBRARY_TYPE} ${coreneuron_all_headers} ${coreneuron_all_templates}
+                       ${coreneuron_all_c_files} ${cudacorenrn_objs} ${MOD2C_STDMECH_OUTPUTS})


Suggested change

${coreneuron_all_c_files} ${cudacorenrn_objs} ${MOD2C_STDMECH_OUTPUTS})

${coreneuron_all_c_files} ${MOD2C_STDMECH_OUTPUTS})

ferdonline · 2019-02-26T22:14:53Z

coreneuron/utils/reports/nrnreport.h

@@ -68,7 +68,7 @@ struct ReportConfiguration {
    double start;                           // start time of report
    double stop;                            // stop time of report
    int num_gids;                           // total number of gids
-    size_t buffer_size;                     // hint on buffer size used for this report
+    int buffer_size;                     // hint on buffer size used for this report


Suggested change

int buffer_size; // hint on buffer size used for this report

int buffer_size; // hint on buffer size used for this report

pramodk

@ferdonline : there is one little issue I faced with GPU build described in #141. But that seems unrelated to this PR and I will take of this next.

Looking cmake & nrnivmodl-core, it doesn't seem like clean workflow and lot to improve. But I will keep energy for v2 refactoring :)

…#133) * Fixing nrnivmodl-core: dir creation, shbang * Remove unnecessary mkdir step, create core sub-directory under x86_64 * Avoiding duplicate symbols when using nrnivmodl-core * Generating libcorenrnmech featuring solve_core() * Create an intermediate library (corenrnmech, similar to the process of nrnivmodl-core) which contains the extra mechanisms and a solve_core() launcher, besides all dependent registration/hook routines (mod_func.cpp and dimplic.cpp) * Recompile kinderiv dependencies * Install libcudacoreneuron; link missing * Set rpath on libcorenrnmech * Fix for GPU. Simplified build and fixed link flag * Fixing rpath Origin in Mac and added deploy script * Remove Wall flags from default compilation * Cleanup debug messages in GPU implementation

…BlueBrain/CoreNeuron#133) * Fixing nrnivmodl-core: dir creation, shbang * Remove unnecessary mkdir step, create core sub-directory under x86_64 * Avoiding duplicate symbols when using nrnivmodl-core * Generating libcorenrnmech featuring solve_core() * Create an intermediate library (corenrnmech, similar to the process of nrnivmodl-core) which contains the extra mechanisms and a solve_core() launcher, besides all dependent registration/hook routines (mod_func.cpp and dimplic.cpp) * Recompile kinderiv dependencies * Install libcudacoreneuron; link missing * Set rpath on libcorenrnmech * Fix for GPU. Simplified build and fixed link flag * Fixing rpath Origin in Mac and added deploy script * Remove Wall flags from default compilation * Cleanup debug messages in GPU implementation CoreNEURON Repo SHA: BlueBrain/CoreNeuron@8fa8c48

ferdonline assigned pramodk Feb 18, 2019

ferdonline requested a review from pramodk February 18, 2019 15:53

ferdonline unassigned pramodk Feb 18, 2019

ferdonline requested a review from nrnhines February 18, 2019 17:02

ferdonline force-pushed the nrnivmodl_core branch 2 times, most recently from 236719a to 3c6eaee Compare February 21, 2019 12:56

ferdonline changed the title ~~Fixing nrnivmodl-core: dir creation, shbang, ;-I~~ Fixing nrnivmodl-core: Avoid redefined symbols + script startup fixes Feb 21, 2019

ferdonline commented Feb 22, 2019

View reviewed changes

ferdonline force-pushed the nrnivmodl_core branch from 705fd62 to ed4fcbb Compare February 26, 2019 16:57

ferdonline commented Feb 26, 2019

View reviewed changes

ferdonline and others added 16 commits February 26, 2019 21:19

Fixing nrnivmodl-core: dir creation, shbang, ;-I

b13b0b1

Drop unnecessary mkdir step

eba50f7

Avoiding duplicate symbols when using nrnivmodl-core

324dc0b

No longer first-found functions in linking. We now strip scoptmath from the core and link together with generated mods when creating coreneuron_app or special-core

Fix: -DADDITIONAL_MECHS to be passed to mech lib

0d856e1

cosmetics

84a152a

minot text update

9454d6d

Add missing kinderiv dependency

beecc0b

Fix incorporation of scopmath to mechlib in linux

4c1c6cf

Install libcorenrnmech for coreneuron_exe to work properly

0083a37

Disable new-dtags with newer linkers

bf85588

Fix issues with PGI compiler

9a71127

disable dtags again

4f6fc69

Install libcudacoreneuron; link missing

83f704c

Allowing static corenrnmech

387c7fe

ferdonline and others added 5 commits February 26, 2019 21:20

Fix for GPU. Simplified build and fixed link flag

4dd6503

Pretty UI :)

7a427ee

Configurable parallelism

cd17377

Revert cuda_wrap_srcs as cuda routines doesnt get linked properly

3a09d20

pramodk force-pushed the nrnivmodl_core branch from c1355a0 to 3a09d20 Compare February 26, 2019 20:31

pramodk and others added 2 commits February 26, 2019 22:29

Remove Wall flags from default compilation

8293f1e

Restructure and warning fix

1b0f3c0

ferdonline commented Feb 26, 2019

View reviewed changes

Cleanup debug messages in GPU implementation

0ea03fb

pramodk approved these changes Feb 26, 2019

View reviewed changes

pramodk merged commit 8fa8c48 into master Feb 26, 2019

ferdonline mentioned this pull request Sep 8, 2020

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

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixing nrnivmodl-core: Avoid redefined symbols + script startup fixes #133

Fixing nrnivmodl-core: Avoid redefined symbols + script startup fixes #133

ferdonline commented Feb 18, 2019 •

edited

Loading

ferdonline commented Feb 21, 2019

ferdonline commented Feb 21, 2019

ferdonline Feb 22, 2019

pramodk Feb 22, 2019

ferdonline Feb 22, 2019

pramodk commented Feb 22, 2019

ferdonline commented Feb 22, 2019

ferdonline Feb 26, 2019

ferdonline Feb 26, 2019

ferdonline Feb 26, 2019

pramodk left a comment

	${coreneuron_all_c_files} ${cudacorenrn_objs} ${MOD2C_STDMECH_OUTPUTS})
	${coreneuron_all_c_files} ${MOD2C_STDMECH_OUTPUTS})

	int buffer_size; // hint on buffer size used for this report
	int buffer_size; // hint on buffer size used for this report

Fixing nrnivmodl-core: Avoid redefined symbols + script startup fixes #133

Fixing nrnivmodl-core: Avoid redefined symbols + script startup fixes #133

Conversation

ferdonline commented Feb 18, 2019 • edited Loading

ferdonline commented Feb 21, 2019

ferdonline commented Feb 21, 2019

ferdonline Feb 22, 2019

Choose a reason for hiding this comment

pramodk Feb 22, 2019

Choose a reason for hiding this comment

ferdonline Feb 22, 2019

Choose a reason for hiding this comment

pramodk commented Feb 22, 2019

ferdonline commented Feb 22, 2019

ferdonline Feb 26, 2019

Choose a reason for hiding this comment

ferdonline Feb 26, 2019

Choose a reason for hiding this comment

ferdonline Feb 26, 2019

Choose a reason for hiding this comment

pramodk left a comment

Choose a reason for hiding this comment

ferdonline commented Feb 18, 2019 •

edited

Loading