Skip to content

Psolve direct#1192

Merged
pramodk merged 73 commits into
masterfrom
psolve-direct
Aug 20, 2021
Merged

Psolve direct#1192
pramodk merged 73 commits into
masterfrom
psolve-direct

Conversation

@nrnhines

@nrnhines nrnhines commented Apr 17, 2021

Copy link
Copy Markdown
Member

ParallelContext.psolve(tstop) behavior for CoreNEURON direct mode is same as NEURON only.
This means CoreNEURON direct mode does not do its own version of finitialize().
On entry to CoreNEURON, all state needed to continue a simulation transfer is copied from NEURON (including Event Queue).
On exit from CoreNEURON all state needed to continue the simulation from NEURON is copied from CoreNEURON (including EventQueue)

Closes #1066 #826
This PR is associated with PR's for BlueBrain/CoreNeuron#528 neuronsimulator/testcorenrn#5 BlueBrain/mod2c#63

For coverage, using:

cmake .. -G Ninja -DCMAKE_INSTALL_PREFIX=install -DNRN_ENABLE_RX3D=OFF -DCMAKE_BUILD_TYPE=Debug -DIV_DIR=$HOME/neuron/ivcmake/build/install -DNRN_ENABLE_TESTS=ON -DNRN_ENABLE_CORENEURON=ON -DNRN_ENABLE_COVERAGE=ON -DNRN_COVERAGE_FILES='src/nrncvode/netcvode.cpp;src/nrniv/netpar.cpp;src/nrniv/nrncore_write.cpp;src/nrniv/nrncore_write/callbacks/nrncore_callbacks.cpp;src/nrniv/nrncore_write/data/cell_group.cpp;src/nrniv/nrncore_write/io/nrncore_io.cpp;src/nrniv/vrecord.cpp;external/coreneuron/coreneuron/io/core2nrn_data_return.cpp;external/coreneuron/coreneuron/io/nrn_setup.cpp;external/coreneuron/coreneuron/io/phase2.cpp;external/coreneuron/coreneuron/mechanism/mech/enginemech.cpp;external/coreneuron/coreneuron/network/netcvode.cpp;external/coreneuron/coreneuron/sim/finitialize.cpp'
ninja install
ninja cover_begin
ninja test
ninja cover_html

Overall coverage rate:
  lines......: 50.0% (3118 of 6242 lines)
  functions..: 47.0% (379 of 807 functions)
View in browser at file:///home/hines/neuron/psolve/build/html/index.html

But note that coverage is not reaching into external/coreneuron files.
As of 26.05.2021 the only missing coverage of modified code in the NEURON part is

nrncore_callbacks.cpp
718 case DiscreteEventType: { // 0
720 case TstopEventType: { // 1
762 weight2intdata[wt].push_back(iloc_wt);
775 all the case PreSynType: { // 4
879 nothing is not movable
883 nothing for void core2nrn_SelfEvent_event(int tid, double td,

@nrnhines nrnhines self-assigned this Apr 17, 2021
@nrnhines nrnhines marked this pull request as draft April 17, 2021 18:50
@nrnhines nrnhines mentioned this pull request Apr 17, 2021
1 task
@codecov-commenter

codecov-commenter commented Apr 18, 2021

Copy link
Copy Markdown

Codecov Report

Merging #1192 (cb0c03a) into master (0ead128) will increase coverage by 0.33%.
The diff coverage is 91.76%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1192      +/-   ##
==========================================
+ Coverage   33.07%   33.40%   +0.33%     
==========================================
  Files         569      570       +1     
  Lines      108957   109248     +291     
==========================================
+ Hits        36037    36499     +462     
+ Misses      72920    72749     -171     
Impacted Files Coverage Δ
src/ivoc/ivocrand.cpp 25.72% <0.00%> (-0.77%) ⬇️
src/nrncvode/netcon.h 36.36% <ø> (+13.63%) ⬆️
src/nrniv/vrecord.cpp 39.91% <ø> (ø)
src/nrnoc/nrn_ansi.h 100.00% <ø> (ø)
src/oc/nrnran123.cpp 72.85% <ø> (ø)
...rniv/nrncore_write/callbacks/nrncore_callbacks.cpp 93.79% <92.13%> (-1.22%) ⬇️
src/nrncvode/netcvode.cpp 40.57% <96.87%> (+0.71%) ⬆️
share/lib/python/neuron/coreneuron.py 75.86% <100.00%> (+0.42%) ⬆️
src/nmodl/nocpout.cpp 79.47% <100.00%> (-0.41%) ⬇️
src/nmodl/parsact.cpp 31.57% <100.00%> (+0.22%) ⬆️
... and 24 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0ead128...cb0c03a. Read the comment docs.

nrnhines added 13 commits April 25, 2021 07:50
Current HEAD b9e3eb3b of coreneuron psolve-direct has link problems. e.g.
 => Binary creating x86_64/special-core
x86_64/libcorenrnmech.so: undefined reference to `coreneuron::point_register_mech(char const**, void (*)(double*, int*, int), void (*)(coreneuron::NrnThread*, coreneuron::Memb_list*, int), void (*)(coreneuron::NrnThread*, coreneuron::Memb_list*, int), void (*)(coreneuron::NrnThread*, coreneuron::Memb_list*, int), void (*)(coreneuron::NrnThread*, coreneuron::Memb_list*, int), int, void* (*)(), void (*)(), int)'
collect2: error: ld returned 1 exit status
  Contains bug but idea is basically sound.
  deferred_type2artdata_ replaced by deferred_type2artml_
to properly figure out the Point_process* from type and
mechanism index.
  Begin extending tests involving psolve with CoreNERUON to verify
that psolve is restartable.
@nrnhines

nrnhines commented May 23, 2021

Copy link
Copy Markdown
Member Author

For testcorenrn, sequence of coreneuron psolve (mode 2) is failing for:

	 42 - testcorenrn_bbcore::compare_results (Failed)
	 47 - testcorenrn_gf::compare_results (Failed)
	 56 - testcorenrn_vecplay::compare_results (Failed)
	 61 - testcorenrn_watch::compare_results (Failed)

However half nrn psolve (tstop/2) followed by coreneuron psolve (tstop) using mode(1) is only failing for

47 - testcorenrn_gf::compare_results (Failed)

The later suggest a lacuna in nrn -> corenrn. And the mode 2 failures suggest missing transfers in the corenrn->nrn direction.

nrnhines added 2 commits May 23, 2021 11:46
  int nrn_random123_setseq(Rand* r, uint32_t seq, char which);
  int nrn_random123_getseq(Rand* r, uint32_t* seq, char* which);
Update external/coreneuron
Extend a couple tests for psolve.
  This is needed at least to update nrnran123 sequence state
in order to be able to continue psolve on either side.
Updated coreneuron and testcorenrn.
@nrnhines

Copy link
Copy Markdown
Member Author

One remaining error for existing tests. Notes for manual testing.

With default_var("arg_psolve_test_mode", 2) in defvar.hoc
61 - testcorenrn_watch::compare_results (Failed)  
Test #58: testcorenrn_watch::coreneuron_cpu_online   
ctest -VV -R testcorenrn_watch::coreneuron_cpu_online
"mpiexec" "-n" "2" "--oversubscribe"
 "special" "-mpi" "-c" "arg_tstop=100" "-c" "arg_coreneuron=1" "testwatch.hoc"
In psolve/build/test/testcorenrn_watch/coreneuron_cpu_online

Not copying WATCH state back to nrn.

Note that ninja install suffices after changing a hoc or mod file in psolve/external/tests/testcorenrn

@nrnhines

nrnhines commented Aug 7, 2021

Copy link
Copy Markdown
Member Author

test_netmove_py
CoreNEURON w/ NMODL on CPU
Mode 0 fails with (self.syn.n_netmove different between NEURON and CoreNEURON(NMODL):

This is fixed by the change to nmodl mentioned by the above commit 293985b
BlueBrain/nmodl#714

@olupton

olupton commented Aug 9, 2021

Copy link
Copy Markdown
Collaborator

Am I correct in thinking that everything is now working wrt CoreNEURON + mod2c and no GPU?

I think you're right. In the latest CoreNEURON CI run (here) everything is OK on CPU, both with mod2c and nmodl. There are still GPU-specific failures to investigate:

The following tests FAILED:
	 15 - coreneuron_modtests::test_netmove_py (Failed)
	 16 - coreneuron_modtests::test_watchrange_py (Failed)

(the same tests fail with both mod2c and nmodl when GPU support is enabled)

Is there anything I can do to help with GPU issues?

I'm not sure. I hope to pick this back up this week so I should have a better idea soon!

Comment thread test/coreneuron/test_direct.py Outdated

@olupton olupton left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good. I didn't stare at every single line of code, but I scanned through and I think we have done a lot of CI / interactive testing of these branches already.

@alexsavulescu

Copy link
Copy Markdown
Member

@nrnhines we need to merge this: neuronsimulator/testcorenrn#5 and then update the submodule

@alexsavulescu

Copy link
Copy Markdown
Member

@pramodk is making final retouches to CoreNEURON PR and we will update the submodule after the merge.

@nrnhines

Copy link
Copy Markdown
Member Author

@pramodk Time to squash and merge? (and I will clean up the commit message :)

@alexsavulescu

Copy link
Copy Markdown
Member

still need to merge CoreNEURON PR and update submodule here 🚀

pramodk added a commit to BlueBrain/CoreNeuron that referenced this pull request Aug 20, 2021
…same as NEURON only (#528)

* This means:
  - CoreNEURON direct mode does not do its own version of finitialize().
  - On entry to CoreNEURON, all state needed to continue a simulation
    is copied from NEURON (including Event Queue).
  - On exit from CoreNEURON all state needed to continue the simulation
    in NEURON is copied from CoreNEURON (including EventQueue)
* Implementation for copying event queue back to NEURON.
* This pull request is associated with neuronsimulator/nrn#1192
* Improve GPU TrajectoryRequests support: any variable (voltage, imembrane
  or any mechanism data) can be recorded now.
* Add extra #pragma acc wait while recording values from GPU
    - drop update_{fast_imem,voltage}_from_gpu() methods, those values are now
      copied by the generic trajectory handling.
    - add comment explaining why this version may be quite slow, and how it
      can be improved.
* Updated nmodl to latest master to fix the various issues (CPU & GPU)
* Check the overflow of NetSendBuffer_t from GPU execution
   - See rationale for this change in BlueBrain/mod2c/pull/68
   - Update the mod2c submodule for CI to pick change of BlueBrain/mod2c/pull/68
* Fixed NEURON_BRANCH setting issue in the CI
* Update PatternStim for direct mode. No special treatment except share Info

Fixes #416

Co-authored-by: Alexandru S�<83>vulescu <alexandru.savulescu@epfl.ch>
Co-authored-by: Michael Hines <michael.hines@yale.edu>
Co-authored-by: Olli Lupton <oliver.lupton@epfl.ch>
Co-authored-by: Pramod Kumbhar <pramod.s.kumbhar@gmail.com>
Co-authored-by: Ioannis Magkanaris <iomagkanaris@gmail.com>
Co-authored-by: Nicolas Cornu <nicolac76@yahoo.fr>
@pramodk pramodk merged commit e9ef741 into master Aug 20, 2021
@pramodk

pramodk commented Aug 20, 2021

Copy link
Copy Markdown
Member

@pramodk Time to squash and merge? (and I will clean up the commit message :)

@nrnhines: Oh sorry! I didn't see this. I had already prepared commit messages as I was eager to merge this before our meeting 😄. I hope I have summarised the changes well enough in e9ef741.

@nrnhines

Copy link
Copy Markdown
Member Author

@pramodk No problem. It is an excellent commit message!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support / Flexibility for psolve with CoreNEURON CoreNEURON direct memory mode. Incremental simulation

6 participants