callback for returning spikes from CORENEURON #692

alexsavulescu · 2020-08-03T16:11:11Z

NEURON facing callback: when we record all or some output spikes (i.e pc.spike_record( -1) or pc.spike_record(gidlist)) provide a callback so that CORENEURON can returnspike vectors.

In link with : BlueBrain/CoreNeuron#354

nrnhines · 2020-08-03T18:34:21Z

src/nrniv/netpar.cpp

-			ps->record(spikevec, gidvec, ps->output_index_);
-		}
-	}}}
+        all_spiketvec = spikevec;


We should consider reference counting all_spiketvec and all_spikegidvec through their obj_ field (if obj_ is non NULL).
In Vector.record and Vector.play this is accomplished through a more complicated InterViews Observer/Observable protocol but I don't recommend using that.

Do you mean if all_spiketvec. obj_ is not null then delete it first before assigning? or something else? (Alex might have better idea about this)

I believe Michael is referring to the usage of underlying vector, when we come back from CORENEURON make sure that this vector is still used (refcount > 0).

pramodk

few comments to understand the implementation.

pramodk · 2020-08-03T22:58:01Z

src/nrniv/netpar.cpp

-			ps->record(spikevec, gidvec, ps->output_index_);
-		}
-	}}}
+        all_spiketvec = spikevec;


Do you mean if all_spiketvec. obj_ is not null then delete it first before assigning? or something else? (Alex might have better idea about this)

pramodk · 2020-08-03T23:02:02Z

src/nrniv/netpar.cpp

+    if(all_spiketvec != nullptr && all_spikegidvec != nullptr) {
+        all_spiketvec->resize(spiketvec.size());
+        all_spikegidvec->resize(spikegidvec.size());
+        assert(all_spiketvec->capacity() == all_spikegidvec->capacity());
+        for(int i = 0; i < all_spiketvec->capacity(); ++i ) {
+            all_spiketvec->elem(i) = spiketvec[i];
+            all_spikegidvec->elem(i) = spikegidvec[i];
+        }


again, I don't know the code but my undemanding was that: we will iterate every spikegidvec and then deliver event to respective PreSyn object with the same GID. This way the implementation work if PreSyns are using underlying different vectors.

I didn't understand fully how global all_spiketvec solution works here.

I'm not sure we need to deliver events, since we are in "record" mode?

On spike_record, either we have gid=-1 for all gids or a gidlist, which would normally target a subset of all gids?
The deal with different vectors is not as straightforward. Say we record spikes for a subset of gids. On spikes return from CORENEURON we would have all gids spike data. We can match underlying different vectors, but then we need to consider writing out.dat for the gids that were not recorded in NEURON.

We did discuss the idea of being generic with respect to the ways NEURON can have different vectors for recording spikes (and not even necessarily use gids but a user defined identifier). I don't remember if spikes are recorded on the CoreNEURON side when gid < 0 (the negative number can potentially identify a PreSyn without a gid) and just discarded if out.dat is written. But I'm not averse to the direct implementation Alex provided. It just may mean that a NEURON spike raster may have or be missing spike trains depending on whether CoreNEURON did the simulation or NEURON did the simulation. But I am ok with only supporting the NEURON case pc.spike_record(-1). Might be a good idea to comment about this limited support in the implementation.

We also discussed about a callback not being needed and just use a direct call into NEURON. But I realize not that would be a problem if CoreNEURON did not have NEURON in memory (eg. file transfer mode).

Ok so discussed things through with Pramod. Keeping pc.spike_record(-1) code as optimisation (and apparently that would the regular case), and added code to handle pc.spike_record(gidlist).

Furthermore, we also cover the case of using pc.spike_record(-1) and then pc.spike_record(gidlist) by invalidating all_*vec pointers

src/nrniv/netpar.cpp

pramodk · 2020-08-04T19:00:27Z

looks good to me as well !

pramodk · 2020-08-05T23:56:20Z

@alexsavulescu : I took liberty to push change fafce52 to this PR. Now the issue is related to setting test:

9: numprocs=2
9: Assertion failed: file /gpfs/bbp.cscs.ch/project/proj16/kumbhar/pramod_scratch/nrn/src/nrniv/netpar.cpp, line 1112
9: 1 NEURON: gid2out_->find(gid, ps)
9: 1  near line 0
9: 1  create soma
9:             ^
9:         1 ParallelContext[0].spike_record(1, ..., ...)
9: Traceback (most recent call last):
9:   File "/gpfs/bbp.cscs.ch/project/proj16/kumbhar/pramod_scratch/nrn/test/coreneuron/test_spikes.py", line 70, in <module>
9:     test_spikes()
9:   File "/gpfs/bbp.cscs.ch/project/proj16/kumbhar/pramod_scratch/nrn/test/coreneuron/test_spikes.py", line 54, in test_spikes
9:     corenrn_all_spike_gids )
9: RuntimeError: hoc error
9: MPT ERROR: MPI_COMM_WORLD rank 1 has terminated without calling MPI_Finalize()
9: 	aborting job
1/1 Test #9: coreneuron_mpi_spikes_py .........***Failed    1.76 sec

cc: @nrnhines

…spikes

alexsavulescu · 2020-08-06T09:45:26Z

@pramodk I followed rxd strategy and used mpi4py.
From test/rxd/conftest.py :

    # to use NEURON with MPI, mpi4py must be imported first.
    if request.config.getoption("--mpi"):
        from mpi4py import MPI  # noqa: F401

pramodk

bit of mystery about failure with python+internal mpi but this looks good for merge now!

* Fixes compilation issue when the operator() was declared as const while it changed some of values outside of its scope in the sympy solver * Use DefUseAnalyzeVisitor to find out whether the operator() is Using or Defining any of the variables outside its scope * Update DefUseAnalyzeVisitor to be able to process LOCAL variables as well, given precedence to GLOBAL/RANGE variables, when they have the same name * Added related unit tests * Fix codegen issue in CONSTRUCTOR and DESTRUCTOR blocks code when compiled for CoreNEURON Fixes #691 FIxes #692

callback for returning all spikes from CORENEURON

80c400f

alexsavulescu marked this pull request as draft August 3, 2020 16:11

alexsavulescu mentioned this pull request Aug 3, 2020

Return all spikes vectors to NEURON BlueBrain/CoreNeuron#354

Merged

nrnhines reviewed Aug 3, 2020

View reviewed changes

pramodk reviewed Aug 3, 2020

View reviewed changes

alexsavulescu force-pushed the all_spikes branch from 12179f7 to 11418db Compare August 4, 2020 08:06

callback return value on successful data copy

b470020

alexsavulescu force-pushed the all_spikes branch from 11418db to b470020 Compare August 4, 2020 08:11

handle general spike vectors transfer capability

1413d94

alexsavulescu commented Aug 4, 2020

View reviewed changes

src/nrniv/netpar.cpp Show resolved Hide resolved

alexsavulescu commented Aug 4, 2020

View reviewed changes

src/nrniv/netpar.cpp Show resolved Hide resolved

Merge branch 'master' into all_spikes

32c6de6

alexsavulescu changed the title ~~callback for returning all spikes from CORENEURON~~ callback for returning spikes from CORENEURON Aug 4, 2020

alexsavulescu added 5 commits August 5, 2020 15:56

callback for returning all spikes from CORENEURON

c5b8de7

callback return value on successful data copy

a117eb6

handle general spike vectors transfer capability

53a7fe4

testing for spikes return

7824e10

pull up latest CORENEURON for spikes

73d8988

alexsavulescu marked this pull request as ready for review August 5, 2020 14:13

alexsavulescu force-pushed the all_spikes branch 5 times, most recently from e210224 to 25353b3 Compare August 5, 2020 19:44

Merge branch 'all_spikes' of github.com:neuronsimulator/nrn into all_…

d5935ab

…spikes

alexsavulescu force-pushed the all_spikes branch from fafce52 to d5935ab Compare August 6, 2020 08:56

pramodk approved these changes Aug 6, 2020

View reviewed changes

nrnhines merged commit cbabda2 into master Aug 8, 2020

nrnhines deleted the all_spikes branch August 8, 2020 12:25

alexsavulescu mentioned this pull request Aug 10, 2020

Accessing recorded spikes via direct memory interface with NEURON BlueBrain/CoreNeuron#340

Closed

alexsavulescu mentioned this pull request Dec 8, 2020

build fixes in master that should get cherry-picked into release/7.8.1 #859

Closed

alexsavulescu mentioned this pull request Apr 26, 2021

Create Release Notes for 8.0 releases #1211

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

callback for returning spikes from CORENEURON #692

callback for returning spikes from CORENEURON #692

alexsavulescu commented Aug 3, 2020 •

edited

Loading

nrnhines Aug 3, 2020

pramodk Aug 3, 2020

alexsavulescu Aug 4, 2020

pramodk left a comment

pramodk Aug 3, 2020

pramodk Aug 3, 2020

alexsavulescu Aug 4, 2020

nrnhines Aug 4, 2020

alexsavulescu Aug 4, 2020

alexsavulescu Aug 4, 2020

pramodk commented Aug 4, 2020

pramodk commented Aug 5, 2020

alexsavulescu commented Aug 6, 2020

pramodk left a comment

callback for returning spikes from CORENEURON #692

callback for returning spikes from CORENEURON #692

Conversation

alexsavulescu commented Aug 3, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pramodk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pramodk commented Aug 4, 2020

pramodk commented Aug 5, 2020

alexsavulescu commented Aug 6, 2020

pramodk left a comment

Choose a reason for hiding this comment

alexsavulescu commented Aug 3, 2020 •

edited

Loading