In memory model transfer between neuron and coreneuron #114

pramodk · 2018-09-13T18:32:12Z

Merging first draft from Michael to transfer neuron model to coreneuron with DMA

* move coreneuron to coreneuron namespace * fix test to comply to new changes * fix c++ link with mod2c generated files

fixing VERBATIM blocks

…-skip-mpi-finalize command line option

… from inlinin (functions are not getting inlined with cpp)

The namespaces were not correctly nested.

This is the first of a series of transformations to allow CoreNeuron to be embedded in NEURON and simulate the NEURON model without using data files. This change reads the bbcore_mech.dat file as an ifstream or can callback, if embedded, to get the bbcore_mech.dat file information directly from NEURON via a stringstream. In both cases the information is handled by the same mk_mech(std::istream&) function. The binary sentinal value for determining Endian consistency has been removed from bbcore_mech.dat and into a binary file called byteswap1.dat bbcore_write_version has been bumped to 1.2

still read from files). On the NEURON side, the coreneuron library is loaded via the CORENEURONLIB environment variable and a coreneuron simulation is loaded and run via ParallelContext.nrncore_run()

ringtest is not producing good raster.

number of threads, and whether gaps and mpi are being used.

Should be a space separated list of args that would normally be used in launching coreneuron_exec. But no need to pass -mpi.

it is set to nthread.

pramodk · 2018-09-13T18:32:45Z

coreneuron/nrniv/global_vars.cpp

+#include "coreneuron/nrniv/nrn2core_direct.h"
+
+void* (*nrn2core_get_global_dbl_item_)(void*, const char*& name, int& size, double*& val);
+int (*nrn2core_get_global_int_item_)(const char* name);


can we transfer these to single header? (nrn2core_direct.h)

I thought they needed a separare declaration in the header (extern) and then a definition in
some file (memory to hold the function pointer)

pramodk · 2018-09-13T18:34:05Z

coreneuron/nrniv/main1.cpp

+        }
+    }
+    argc += use_mpi ? 2 : 1;  // corenrn -mpi or just corenrn
+    char** argv = new char*[argc];


corresponding delete missing?

pramodk · 2018-09-13T18:34:56Z

coreneuron/nrniv/main1.cpp

+    argc = 0;
+    argv[argc++] = strdup("corenrn");
+    if (use_mpi) {
+        argv[argc++] = strdup("-mpi");


corresponding free missing?

Yes. In general, though, on return from nrncore_run it is not clear at the moment how much access NEURON needs to the coreneuron results. .eg. perhaps there should be an inverse
to nrnbbcore_write. Or, as we mentioned in the paper a series of accessor functions of various grain.

pramodk · 2018-09-13T18:35:10Z

coreneuron/nrniv/main1.cpp

+        int sp = isspace(c) ? 1 : ((c == '\0') ? 1 : 0);
+        if (inarg && sp) {  // start whitespace following previous arg
+            inarg = 0;
+            argv[argc++] = strndup(arg + first, i - first);


corresponding free missing?

It is safe to free all argv and argv itself on return from nrncore_run.

pramodk · 2018-09-13T18:35:39Z

coreneuron/nrniv/main1.cpp

+    printf("arg: %s\n", arg);
+    for (i = 0; i < argc; ++i) {
+        printf("%d %s\n", i, argv[i]);
+    }


only rank 0 should print?

yes. unfortunately, coreneuron has not started mpi yet and nrniv did not pass its version of
nrnmpi_myid to nrncore_run

These were for debugging and can be removed

pramodk · 2018-09-13T18:36:20Z

coreneuron/nrniv/mk_mech.cpp


+extern "C" {
+void (*nrn2core_mkmech_info_)(std::ostream&);
+}


can this be moved to nrn2core_direct.h?

I thought it needed a unique definition

pramodk · 2018-09-13T18:37:38Z

coreneuron/nrniv/nrn_setup.cpp

+                                       int& sz,
+                                       double*& yvec,
+                                       double*& tvec);
+


if these are declared in nrn2core_direct.h, why we need to declare here again?

they are all extern. the file is included in multiple places.

pramodk · 2018-09-13T18:41:44Z

I think we should run valgrind to make sure all memory is getting freed in embedded mode.

nrnhines

Subject to my detailed comments you can decide
whether to merge

nrnhines · 2018-09-13T19:13:49Z

With respect to valgrind, that is an excellent idea. Note that the ability to make multiple calls to
pc.nrncore_run(string) has not been thought through yet.

pramodk · 2018-10-01T22:11:47Z

@nrnhines : with last two commits I have fixed memory leaks with valgrind. I am not sure about removing event from queue:

==53751== 96 bytes in 2 blocks are definitely lost in loss record 79 of 118
==53751==    at 0x100099616: malloc (in /usr/local/Cellar/valgrind/3.13.0/lib/valgrind/vgpreload_memcheck-amd64-darwin.so)
==53751==    by 0x10135BE0D: operator new(unsigned long) (in /usr/lib/libc++abi.dylib)
==53751==    by 0x1000F668B: coreneuron::TQueue<(coreneuron::container)1>::insert(double, void*) (tqueue.ipp:243)
==53751==    by 0x1000F5294: coreneuron::NetCvode::event(double, coreneuron::DiscreteEvent*, coreneuron::NrnThread*) (netcvode.cpp:251)
==53751==    by 0x100101331: coreneuron::NetParEvent::send(double, coreneuron::NetCvode*, coreneuron::NrnThread*) (netpar.cpp:132)
==53751==    by 0x100101BEF: coreneuron::nrn_spike_exchange_init() (netpar.cpp:306)
==53751==    by 0x10011C69E: coreneuron::checkpoint_initialize() (nrn_checkpoint.cpp:860)
==53751==    by 0x1000D3F7A: solve_core (main1.cpp:396)
==53751==    by 0x100001F81: main (coreneuron.cpp:33)

Obviously this is not related to this branch though.

- when direct transfer mode is used - memory leaks from previous version

This is the first of a series of transformations to allow CoreNeuron to be embedded in NEURON and simulate the NEURON model without using data files. This change reads the bbcore_mech.dat file as an ifstream or can callback, if embedded, to get the bbcore_mech.dat file information directly from NEURON via a stringstream. In both cases the information is handled by the same mk_mech(std::istream&) function. - binary sentinel value for determining Endian consistency has been removed from bbcore_mech.dat and into a binary file called byteswap1.dat - bbcore_write_version has been bumped to 1.2 - direct transfer of globals.dat info in place - update test dataset - if OMP_NUM_THREADS is not set then it is set to nthread.

…eNeuron#114) This is the first of a series of transformations to allow CoreNeuron to be embedded in NEURON and simulate the NEURON model without using data files. This change reads the bbcore_mech.dat file as an ifstream or can callback, if embedded, to get the bbcore_mech.dat file information directly from NEURON via a stringstream. In both cases the information is handled by the same mk_mech(std::istream&) function. - binary sentinel value for determining Endian consistency has been removed from bbcore_mech.dat and into a binary file called byteswap1.dat - bbcore_write_version has been bumped to 1.2 - direct transfer of globals.dat info in place - update test dataset - if OMP_NUM_THREADS is not set then it is set to nthread. CoreNEURON Repo SHA: BlueBrain/CoreNeuron@f2ccf7f

fouriaux and others added 30 commits March 27, 2018 14:45

* move coreneuron toward full c++

283ee1b

* move coreneuron to coreneuron namespace * fix test to comply to new changes * fix c++ link with mod2c generated files

Update mod2c to new commit

a18f77e

fix ivocvect vector compatibility and declarations

d129dbb

fixing core_read and core_write to be out of coreneuron namespace

32c4b1b

fixing VERBATIM blocks

add missing prototype

e80852c

update mod2c

9a9aa18

update mod2c link

7afb97d

update submodule

f6a80aa

fix public API of coreneuron to explose core_psolve

a812e74

add pulbic header

aabed5d

kinderiv.h has prototype declaration for euler methods

a7d0c9b

public header accessible from C, clean up unused arguments

859a6a9

Changes to integrate with neurodamus: psolve_core renamed and added -…

0e17acd

…-skip-mpi-finalize command line option

Merge branch 'master' into full_cpp_interface

ba4105a

mod2c updated

95798d8

W.I.P for fixup openACC version

a7d827b

OpenACC pragma annotation was missing for kernels using euler

ca893d6

cudaMemset wrong pointer fix

992000f

User provided flags should take precedence

9a572b7

Merge remote-tracking branch 'origin/euler_acc' into full_cpp_interface

f639ed7

Removed unnecessary acc declare annotations: actual errors are coming…

18692d9

… from inlinin (functions are not getting inlined with cpp)

Fix issue while compiling with -DENABLE_MPI=OFF

503ff8d

The namespaces were not correctly nested.

Merge branch 'master' into full_cpp_interface

f9facdc

fix mpi build

66f0a4f

Direct transfer of globals.dat info in place.

7c5b70a

Gap junction direct transfer. Change callback prefix to nrn2core_ .

a6d0339

Phase1 direct transfer.

9f97ead

Seems to work with direct transfer of data (but phase2 dat files

642e6f9

still read from files). On the NEURON side, the coreneuron library is loaded via the CORENEURONLIB environment variable and a coreneuron simulation is loaded and run via ParallelContext.nrncore_run()

Direct transfer of dat2 to coreneuron almost works but

fe0cde6

ringtest is not producing good raster.

nrnhines and others added 12 commits May 8, 2018 15:44

Direct transfer works for ringtest.

4bb4cc8

Merge branch 'master' into nrn2coredirect

2867348

Update funding notice with latest HBP and NI grants

a372469

Merge branch 'master' into nrn2coredirect

ca57775

Update test dataset

6747aba

coreneuron_embedded_run does not read any files but needs info about

c969af1

number of threads, and whether gaps and mpi are being used.

Fix coreneuron gap junction direct transfer bugs.

7f769a4

coreneuron_embedded_run takes tstop as an argument.

f53a269

Changed the last arg of coreneuron_embedded_run to const char* arg

89d47ed

Should be a space separated list of args that would normally be used in launching coreneuron_exec. But no need to pass -mpi.

For direct transfer, if OMP_NUM_THREADS is not set then

99d422e

it is set to nthread.

Allow CoreNEURON to send data version to NEURON

34ff7ee

Update Michael's branch with clang-format

92d8101

pramodk commented Sep 13, 2018

View reviewed changes

nrnhines approved these changes Sep 13, 2018

View reviewed changes

Refactor argument parsing logic with strtok

a459e3a

pramodk force-pushed the nrn2coredirect_master branch from 2b823a2 to 184e6bd Compare October 1, 2018 22:08

pramodk force-pushed the nrn2coredirect_master branch from 184e6bd to 9a9e6ca Compare October 1, 2018 22:14

Fix memory leaks :

885d035

- when direct transfer mode is used - memory leaks from previous version

pramodk force-pushed the nrn2coredirect_master branch from 9a9e6ca to 885d035 Compare October 1, 2018 22:16

Merge branch 'master' into nrn2coredirect_master

450409d

pramodk merged commit f2ccf7f into master Oct 3, 2018

pramodk deleted the nrn2coredirect_master branch December 4, 2020 18:14

In memory model transfer between neuron and coreneuron #114

In memory model transfer between neuron and coreneuron #114

Uh oh!

Conversation

pramodk commented Sep 13, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pramodk commented Sep 13, 2018

Uh oh!

nrnhines left a comment

Choose a reason for hiding this comment

Uh oh!

nrnhines commented Sep 13, 2018

Uh oh!

pramodk commented Oct 1, 2018

Uh oh!

Uh oh!