Extend CoreNEURON POINTER transfer to any RANGE variable in a NRN_THREAD #1622

nrnhines · 2022-02-07T23:08:12Z

Prior to this, POINTER was restricted to point to voltage.
This change depends on BlueBrain/CoreNeuron#772

TODO:

Extend POINTER transfer to any RANGE variable in a NRN_THREAD BlueBrain/CoreNeuron#772

Requires bbcore_write_version 1.5
The test requires merge of BlueBrain/CoreNeuron#748

To test (until BlueBrain/CoreNeuron#748 is merged)

git checkout hines/POINTER-to-RANGE
cmake .. -G Ninja -DCMAKE_INSTALL_PREFIX=install -DPYTHON_EXECUTABLE=`which python3` -DNRN_ENABLE_RX3D=OFF -DCMAKE_BUILD_TYPE=Debug  -DNRN_ENABLE_CORENEURON=ON -DNRN_ENABLE_TESTS=ON
(cd ../external/coreneuron/ ; git checkout hines/before-after ; git pull ; git checkout hines/POINTER-to-RANGE ; git pull ; git merge hines/before-after)
ninja install
ctest -R test_pointer_py_cpu

codecov-commenter · 2022-02-26T19:23:23Z

Codecov Report

Merging #1622 (70c9569) into master (4defcc3) will increase coverage by 0.07%.
The diff coverage is 72.72%.

@@            Coverage Diff             @@
##           master    #1622      +/-   ##
==========================================
+ Coverage   45.22%   45.30%   +0.07%     
==========================================
  Files         551      551              
  Lines      111198   111206       +8     
==========================================
+ Hits        50289    50380      +91     
+ Misses      60909    60826      -83

Impacted Files	Coverage Δ
.../nrniv/nrncore_write/callbacks/nrncore_callbacks.h	`100.00% <ø> (ø)`
src/nrniv/nrncore_write/data/cell_group.cpp	`91.44% <ø> (+1.06%)`	⬆️
src/nrniv/nrncore_write/data/cell_group.h	`100.00% <ø> (ø)`
src/nrncvode/netcvode.cpp	`66.49% <14.28%> (+0.30%)`	⬆️
...rniv/nrncore_write/callbacks/nrncore_callbacks.cpp	`95.76% <100.00%> (+0.19%)`	⬆️
src/nrniv/nrncore_write/io/nrncore_io.cpp	`75.36% <100.00%> (+0.45%)`	⬆️
src/nrnmpi/bbsmpipack.cpp	`82.56% <0.00%> (-10.77%)`	⬇️
src/parallel/bbsclimpi.cpp	`50.00% <0.00%> (-8.14%)`	⬇️
src/parallel/bbssrvmpi.cpp	`42.16% <0.00%> (-6.03%)`	⬇️
src/parallel/bbs.cpp	`74.26% <0.00%> (-1.69%)`	⬇️
... and 10 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 4defcc3...70c9569. Read the comment docs.

There is likely a but in WATCH statement when checkpoint with cell-permute 1. So that test is currently turned off.

nrnhines · 2022-03-04T14:37:44Z

f88f823 test does reveal a bug in CoreNEURON checkpointing but it has nothing to do with the WATCH statement either. It has to do with some kind of permutation problem so that the PreSyn is not properly associated with the correct AxialPP instance (at least after checkpoint read). I.e the visible symptom is that gid's for the spike raster file are wrong. But if gids are stored in the AxialPP instance itself as a PARAMETER RANGE variable, and one

NET_RECEIVE(w) {
  if (id > 0) {
    printf("%g %g %g zzz\n", t, id, iaSum)
  }
...

one sees the correct gid in the id range variable associated with the proper time and value. In other words, the first few spikes after a checkpoint restart prints

5.025 124 0.501053 zzz
5.175 111 0.506255 zzz
5.275 126 0.505367 zzz
5.55 104 0.519031 zzz

but the the corresponding spikes in the spike raster out.dat file are

5.025   127
5.175   114
5.275   103
5.55    124

nrnhines · 2022-03-05T12:11:07Z

@pramod This is ready for review with respect to POINTER functionality. However, I would like to explicitly support the handling of multiple BEFORE SETUP blocks in a single mod file. The axial_pp.mod, axial.mod, and axial.inc files can benefit from that in terms of reducing copies of identical code fragments. However it may involve a similar change over on the CoreNEURON side as well for BlueBrain/CoreNeuron#772

pramodk · 2022-03-07T16:31:26Z

@nrnhines : As mentioned in BlueBrain/CoreNeuron#772, lets have a separate PR so that we can get this merged?

nrnhines · 2022-03-07T16:38:05Z

I agree. I'll defer the change to a separate pull request after this is merged.

alexsavulescu

LGTM. Some cosmetic touches.

src/nrniv/nrncore_write/callbacks/nrncore_callbacks.cpp

test/coreneuron/test_pointer.py

…reneuron CI

pramodk · 2022-03-10T18:52:52Z

Just for the record - CircleCI failure seems like a hiccup:

Reading package lists... Done
W: --force-yes is deprecated, use one of the options starting with --allow instead.
E: Failed to fetch http://us-east-1.ec2.ports.ubuntu.com/ubuntu-ports/dists/focal-updates/main/binary-arm64/Packages.xz  File has unexpected size (1137644 != 1139168). Mirror sync in progress? [IP: 34.207.203.110 80]
   Hashes of expected file:
    - Filesize:1139168 [weak]
    - SHA256:83e3736496f0f874e48d6914bae3a62a5f830581db7071bf6520577bd20e7102
    - SHA1:81166858c09120d9bdd1680d1c565cabfe10d16b [weak]
    - MD5Sum:c3057169215bd6bdeef8ddf39effab1b [weak]
   Release file created at: Thu, 10 Mar 2022 17:53:55 +0000
E: Failed to fetch http://us-east-1.ec2.ports.ubuntu.com/ubuntu-ports/dists/focal-updates/universe/binary-arm64/Packages.xz  
E: Some index files failed to download. They have been ignored, or old ones used instead.

Have retriggered the CI.

pramodk

LGTM

pramodk · 2022-03-10T19:58:03Z

test/coreneuron/mod/axial.inc

+NEURON {
+    THREADSAFE
+    RANGE ri, ia, im
+    POINTER pv, pia, pim


@nrnhines : to clarify - now not all POINTERs are invalid in the context of CoreNEURON. Only POINTERS that are used for explicit memory allocation needs to be handled with bbcore_read and bbcore_write. Right? We can add this clarification in #1678.

@pramodk That is correct. That has always been correct for POINTER to voltage.

pramodk · 2022-03-10T20:02:02Z

test/coreneuron/test_pointer.py

+    # sometimes roundoff to %.8g gives different sort.
+    srun("sortspike {}/temp {}/nrn.spk".format(dir, dir))
+
+    srun("sortspike {}/out.dat {}/out.spk".format(dir, dir))
+    srun("cmp {}/out.spk {}/nrn.spk".format(dir, dir))
+
+    cmd = "cat"
+    for i in chkpntdirs:
+        cmd = cmd + " " + i + "/out.dat"
+    srun(cmd + " > " + dir + "/temp")
+    srun("sortspike {}/temp {}/chkptout.spk".format(dir, dir))
+    srun("cmp {}/out.spk {}/chkptout.spk".format(dir, dir))


just a comment: this all looks bash embedded inside python 😅 but I think we can leave this. In #1682 Olli refactor similar test inside CMake but this one doesn't include MPI or similar options. So hopefully we will be able to clean this after 8.1 by bringing coreneuron integrated into neuron.

nrnhines added 5 commits January 23, 2022 09:04

CoreNEURON POINTER transfer to any RANGE variable

bc13be3

logic error about whether there are any trajectories for a thread.

9b99097

update coreneuron latest hines/POINTER-to-RANGE

cd634d4

test of non-voltage POINTER

1c988d5

test of POINTER transfer and coreneuron.cell_permute = 1

99715b5

nrnhines requested review from pramodk, olupton and alexsavulescu February 7, 2022 23:08

nrnhines mentioned this pull request Feb 7, 2022

Extend POINTER transfer to any RANGE variable in a NRN_THREAD BlueBrain/CoreNeuron#772

Merged

olupton and others added 5 commits February 11, 2022 09:55

Update CoreNEURON submodule to latest hines/POINTER-to-RANGE.

7d726c8

Merge branch 'master' into hines/POINTER-to-RANGE

27f7a26

Merge branch 'master' into hines/POINTER-to-RANGE

f8c56d9

Merge branch 'master' into hines/POINTER-to-RANGE

328630d

update coreneuron (awaiting pointer2type for checkpoint and test)

ed29336

update coreneuron

c008bef

pramodk modified the milestones: Frontiers paper, Release v8.1 Feb 28, 2022

Merge branch 'master' into hines/POINTER-to-RANGE

dc8a2a9

nrnhines mentioned this pull request Feb 28, 2022

Rxd sourcevar #1549

Draft

nrnhines added 2 commits March 2, 2022 09:43

update coreneuron. starting a test for file mode transfer.

e24c05d

CoreNEURON file mode checkpoint test for POINTER

f88f823

There is likely a but in WATCH statement when checkpoint with cell-permute 1. So that test is currently turned off.

update coreneuron. cell_permute=1 works for POINTER and checkpoint

61f7205

nrnhines mentioned this pull request Mar 5, 2022

What is a robust way to call sortspike from python? #1675

Closed

nrnhines added 3 commits March 4, 2022 21:41

Augment path used by ctest with ${PROJECT_BINARY_DIR}/bin

7275c8f

update coreneuron

1665fb9

Merge branch 'master' into hines/POINTER-to-RANGE

5c6cd3c

update coreneuron submodule

96415b3

pramodk assigned alexsavulescu Mar 7, 2022

alexsavulescu approved these changes Mar 7, 2022

View reviewed changes

src/nrniv/nrncore_write/callbacks/nrncore_callbacks.cpp Outdated Show resolved Hide resolved

test/coreneuron/test_pointer.py Outdated Show resolved Hide resolved

test/coreneuron/test_pointer.py Outdated Show resolved Hide resolved

alexsavulescu and others added 10 commits March 7, 2022 18:19

Apply suggestions from code review

99f015e

Merge branch 'master' into hines/POINTER-to-RANGE

d41802f

print more information if subprocess.run fails

f7be90f

Merge branch 'master' into hines/POINTER-to-RANGE

f56b7ca

Merge branch 'master' into hines/POINTER-to-RANGE

4a0d025

Fix similar to #1619 for pointer test: sonata env var required for co…

52f5b56

…reneuron CI

Update coreneuron submodule

c823fbf

update coreneuron

70c9569

update coreneuron submodule to latest master

9b1c22d

Merge branch 'master' into hines/POINTER-to-RANGE

9abfdb9

pramodk removed the request for review from olupton March 10, 2022 19:01

pramodk approved these changes Mar 10, 2022

View reviewed changes

nrnhines merged commit d52513a into master Mar 10, 2022

nrnhines deleted the hines/POINTER-to-RANGE branch March 10, 2022 20:18

iomaganaris mentioned this pull request Mar 13, 2022

Removed POINTER compatibility error BlueBrain/nmodl#821

Merged

alexsavulescu mentioned this pull request Mar 22, 2022

NEURON 8.1.0 Release #1719

Closed

15 tasks

pramodk mentioned this pull request Apr 19, 2022

Usage of POINTER variable under PROCEDURE or other blocks result into compilation error (CoreNEURON) #1782

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extend CoreNEURON POINTER transfer to any RANGE variable in a NRN_THREAD #1622

Extend CoreNEURON POINTER transfer to any RANGE variable in a NRN_THREAD #1622

nrnhines commented Feb 7, 2022 •

edited by pramodk

Loading

codecov-commenter commented Feb 26, 2022 •

edited

Loading

nrnhines commented Mar 4, 2022

nrnhines commented Mar 5, 2022 •

edited

Loading

pramodk commented Mar 7, 2022

nrnhines commented Mar 7, 2022

alexsavulescu left a comment

pramodk commented Mar 10, 2022

pramodk left a comment

pramodk Mar 10, 2022

nrnhines Mar 11, 2022

pramodk Mar 10, 2022

Extend CoreNEURON POINTER transfer to any RANGE variable in a NRN_THREAD #1622

Extend CoreNEURON POINTER transfer to any RANGE variable in a NRN_THREAD #1622

Conversation

nrnhines commented Feb 7, 2022 • edited by pramodk Loading

codecov-commenter commented Feb 26, 2022 • edited Loading

Codecov Report

nrnhines commented Mar 4, 2022

nrnhines commented Mar 5, 2022 • edited Loading

pramodk commented Mar 7, 2022

nrnhines commented Mar 7, 2022

alexsavulescu left a comment

Choose a reason for hiding this comment

pramodk commented Mar 10, 2022

pramodk left a comment

Choose a reason for hiding this comment

pramodk Mar 10, 2022

Choose a reason for hiding this comment

nrnhines Mar 11, 2022

Choose a reason for hiding this comment

pramodk Mar 10, 2022

Choose a reason for hiding this comment

nrnhines commented Feb 7, 2022 •

edited by pramodk

Loading

codecov-commenter commented Feb 26, 2022 •

edited

Loading

nrnhines commented Mar 5, 2022 •

edited

Loading