From 4232cab222fe66634e49ad5d8beb4bf47d7c19a9 Mon Sep 17 00:00:00 2001 From: wthun Date: Thu, 22 Aug 2024 14:41:20 +0200 Subject: [PATCH 01/22] increased maximum number of ions per compartment by using std::bitset for bitvector comparisons --- src/nrnoc/eion.cpp | 49 +++++++++++++++++++++++++++++----------------- 1 file changed, 31 insertions(+), 18 deletions(-) diff --git a/src/nrnoc/eion.cpp b/src/nrnoc/eion.cpp index 7933fabe29..5721943836 100644 --- a/src/nrnoc/eion.cpp +++ b/src/nrnoc/eion.cpp @@ -14,6 +14,10 @@ #include #include +#include +#include +#include + #undef hoc_retpushx extern double chkarg(int, double low, double high); @@ -64,7 +68,7 @@ double nrn_ion_charge(Symbol* sym) { void ion_register(void) { /* hoc level registration of ion name. Return -1 if name already - in use and not an ion; and the mechanism subtype otherwise. + in use and not an ion; and the mechanism subtype otherwise. */ char* name; Symbol* s; @@ -401,7 +405,11 @@ The argument `i` specifies which concentration is being written to. It's 0 for exterior; and 1 for interior. */ void nrn_check_conc_write(Prop* p_ok, Prop* pion, int i) { - static long *chk_conc_, *ion_bit_, size_; + const int max_length = 10000; // parametrize? + static std::unique_ptr>> chk_conc_; + static std::unique_ptr>> ion_bit_; + static long size_; + Prop* p; int flag, j, k; if (i == 1) { @@ -409,41 +417,46 @@ void nrn_check_conc_write(Prop* p_ok, Prop* pion, int i) { } else { flag = 0400; } - /* an embarassing hack */ - /* up to 32 possible ions */ - /* continuously compute a bitmap that allows determination - of which models WRITE which ion concentrations */ - if (n_memb_func > size_) { + + // create a vctor of std::bitsets to track ions being written + if (n_memb_func > size_) { // No need to reallocate now? if (!chk_conc_) { - chk_conc_ = (long*) ecalloc(2 * n_memb_func, sizeof(long)); - ion_bit_ = (long*) ecalloc(n_memb_func, sizeof(long)); + chk_conc_ = std::make_unique>>(); + chk_conc_->reserve(2 * n_memb_func); + + ion_bit_ = std::make_unique>>(); + ion_bit_->reserve(n_memb_func); } else { - chk_conc_ = (long*) erealloc(chk_conc_, 2 * n_memb_func * sizeof(long)); - ion_bit_ = (long*) erealloc(ion_bit_, n_memb_func * sizeof(long)); + chk_conc_ = std::make_unique>>(); + chk_conc_->reserve(2 * n_memb_func); + + ion_bit_ = std::make_unique>>(); + ion_bit_->reserve(n_memb_func); for (j = size_; j < n_memb_func; ++j) { - chk_conc_[2 * j] = 0; - chk_conc_[2 * j + 1] = 0; - ion_bit_[j] = 0; + (*chk_conc_)[2 * j] &= 0; + (*chk_conc_)[2 * j + 1] &= 0; + (*ion_bit_)[j] &= 0; } } size_ = n_memb_func; } for (k = 0, j = 0; j < n_memb_func; ++j) { if (nrn_is_ion(j)) { - ion_bit_[j] = (1 << k); + (*ion_bit_)[j] = (1 << k); ++k; - assert(k < sizeof(long) * 8); + assert(k < max_length); } } - chk_conc_[2 * p_ok->_type + i] |= ion_bit_[pion->_type]; + (*chk_conc_)[2 * p_ok->_type + i] |= (*ion_bit_)[pion->_type]; if (pion->dparam[iontype_index_dparam].get() & flag) { /* now comes the hard part. Is the possibility in fact actual.*/ for (p = pion->next; p; p = p->next) { if (p == p_ok) { continue; } - if (chk_conc_[2 * p->_type + i] & ion_bit_[pion->_type]) { + auto rst = (*chk_conc_)[2 * p->_type + i] & (*ion_bit_)[pion->_type]; + if (rst.count() > 0) { char buf[300]; Sprintf(buf, "%.*s%c is being written at the same location by %s and %s", From 5179cc4f34f63c7a965e892530a861a85eab8fd6 Mon Sep 17 00:00:00 2001 From: wthun Date: Fri, 23 Aug 2024 09:41:11 +0200 Subject: [PATCH 02/22] changed vector.reserve to vector.resize to initialize std::bitset elements --- src/nrnoc/eion.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/nrnoc/eion.cpp b/src/nrnoc/eion.cpp index 5721943836..3010f0d5ea 100644 --- a/src/nrnoc/eion.cpp +++ b/src/nrnoc/eion.cpp @@ -422,16 +422,16 @@ void nrn_check_conc_write(Prop* p_ok, Prop* pion, int i) { if (n_memb_func > size_) { // No need to reallocate now? if (!chk_conc_) { chk_conc_ = std::make_unique>>(); - chk_conc_->reserve(2 * n_memb_func); + chk_conc_->resize(2 * n_memb_func); // std::bitset defaults to 0 ion_bit_ = std::make_unique>>(); - ion_bit_->reserve(n_memb_func); + ion_bit_->resize(n_memb_func); } else { chk_conc_ = std::make_unique>>(); - chk_conc_->reserve(2 * n_memb_func); + chk_conc_->resize(2 * n_memb_func); ion_bit_ = std::make_unique>>(); - ion_bit_->reserve(n_memb_func); + ion_bit_->resize(n_memb_func); for (j = size_; j < n_memb_func; ++j) { (*chk_conc_)[2 * j] &= 0; (*chk_conc_)[2 * j + 1] &= 0; From 61162e49ef41bdebab7c13b24fd23001328f37de Mon Sep 17 00:00:00 2001 From: wthun Date: Fri, 23 Aug 2024 09:53:52 +0200 Subject: [PATCH 03/22] cleanup --- src/nrnoc/eion.cpp | 39 ++++++++++++++++----------------------- 1 file changed, 16 insertions(+), 23 deletions(-) diff --git a/src/nrnoc/eion.cpp b/src/nrnoc/eion.cpp index 3010f0d5ea..302615984d 100644 --- a/src/nrnoc/eion.cpp +++ b/src/nrnoc/eion.cpp @@ -406,10 +406,10 @@ exterior; and 1 for interior. */ void nrn_check_conc_write(Prop* p_ok, Prop* pion, int i) { const int max_length = 10000; // parametrize? - static std::unique_ptr>> chk_conc_; - static std::unique_ptr>> ion_bit_; static long size_; + static std::vector> chk_conc_, ion_bit_; + Prop* p; int flag, j, k; if (i == 1) { @@ -418,44 +418,37 @@ void nrn_check_conc_write(Prop* p_ok, Prop* pion, int i) { flag = 0400; } - // create a vctor of std::bitsets to track ions being written - if (n_memb_func > size_) { // No need to reallocate now? - if (!chk_conc_) { - chk_conc_ = std::make_unique>>(); - chk_conc_->resize(2 * n_memb_func); // std::bitset defaults to 0 + /* Create a vector holding std::bitset to track which ions + are being written to the membrane */ + if (n_memb_func > size_) { - ion_bit_ = std::make_unique>>(); - ion_bit_->resize(n_memb_func); - } else { - chk_conc_ = std::make_unique>>(); - chk_conc_->resize(2 * n_memb_func); - - ion_bit_ = std::make_unique>>(); - ion_bit_->resize(n_memb_func); - for (j = size_; j < n_memb_func; ++j) { - (*chk_conc_)[2 * j] &= 0; - (*chk_conc_)[2 * j + 1] &= 0; - (*ion_bit_)[j] &= 0; - } + chk_conc_.resize(2 * n_memb_func); + ion_bit_.resize(n_memb_func); + + for (j = size_; j < n_memb_func; ++j) { + chk_conc_[2 * j].reset(); + chk_conc_[2 * j + 1].reset(); + ion_bit_[j].reset(); } + size_ = n_memb_func; } for (k = 0, j = 0; j < n_memb_func; ++j) { if (nrn_is_ion(j)) { - (*ion_bit_)[j] = (1 << k); + ion_bit_[j] = (1 << k); ++k; assert(k < max_length); } } - (*chk_conc_)[2 * p_ok->_type + i] |= (*ion_bit_)[pion->_type]; + chk_conc_[2 * p_ok->_type + i] |= ion_bit_[pion->_type]; if (pion->dparam[iontype_index_dparam].get() & flag) { /* now comes the hard part. Is the possibility in fact actual.*/ for (p = pion->next; p; p = p->next) { if (p == p_ok) { continue; } - auto rst = (*chk_conc_)[2 * p->_type + i] & (*ion_bit_)[pion->_type]; + auto rst = chk_conc_[2 * p->_type + i] & ion_bit_[pion->_type]; if (rst.count() > 0) { char buf[300]; Sprintf(buf, From 3c01089aed25256899e4f550b41200374e0152bc Mon Sep 17 00:00:00 2001 From: Michael Hines Date: Thu, 19 Sep 2024 11:18:28 -0400 Subject: [PATCH 04/22] test of high mechanism index ions and mechanisms --- test/hoctests/tests/test_many_ions.py | 53 +++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) create mode 100644 test/hoctests/tests/test_many_ions.py diff --git a/test/hoctests/tests/test_many_ions.py b/test/hoctests/tests/test_many_ions.py new file mode 100644 index 0000000000..0f2b4245e6 --- /dev/null +++ b/test/hoctests/tests/test_many_ions.py @@ -0,0 +1,53 @@ +# Test when large number of ion mechanisms exist. + +# Strategy is to create a large number of ion mechanisms before loading +# the neurondemo mod file shared library which will create the ca ion +# along with several mechanisms that write to cai. + +from neuron import h +from platform import machine +import sys + + +# return True if name exists in h +def exists(name): + try: + exec("h.%s" % name) + except: + return False + return True + + +# use up a large number of mechanism indices for ions +nion = 1000 +ion_indices = [int(h.ion_register("ca%d" % i, 2)) for i in range(1, nion+1)] +# gain some confidence they exist +assert exists("ca%d_ion"%nion) +assert (ion_indices[-1] - ion_indices[0]) == (nion - 1) +mt = h.MechanismType(0) +assert mt.count() > nion + +# this test depends on the ca ion not existing at this point +assert exists("ca_ion") is False + +# load neurondemo mod files. That will create ca_ion and provide two +# mod files that write cai +nrnmechlib = ( + "nrnmech.dll" if sys.platform == "win32" else "%s/libnrnmech.so" % machine() +) +# Following Aborts prior to PR#3055 with +# eion.cpp:431: void nrn_check_conc_write(Prop*, Prop*, int): Assertion `k < sizeof(long) * 8' failed. +nrnmechlibname = "%s/demo/release/%s" % (h.neuronhome(), nrnmechlib) +print(nrnmechlibname) +assert h.nrn_load_dll(nrnmechlibname) + +# ca_ion now exists and has a mechanism index > nion +assert exists("ca_ion") +mt = h.MechanismType(0) # new mechanisms do not appear in old MechanismType +mt.select("ca_ion") +assert mt.selected() > nion + +# test if can use the high mechanism index cadifpmp (with USEION ... WRITE cai) along with the high mechanims index ca_ion +s = h.Section() +s.insert("cadifpmp") +h.finitialize(-65) From 7ccc14345756c2be460bc35380f0d4ad41941d80 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 19 Sep 2024 15:44:37 +0000 Subject: [PATCH 05/22] Fix formatting --- src/nrnoc/eion.cpp | 7 +++---- test/hoctests/tests/test_many_ions.py | 4 ++-- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/nrnoc/eion.cpp b/src/nrnoc/eion.cpp index 7a8db092bc..124ebe6430 100644 --- a/src/nrnoc/eion.cpp +++ b/src/nrnoc/eion.cpp @@ -401,7 +401,7 @@ The argument `i` specifies which concentration is being written to. It's 0 for exterior; and 1 for interior. */ void nrn_check_conc_write(Prop* p_ok, Prop* pion, int i) { - const int max_length = 10000; // parametrize? + const int max_length = 10000; // parametrize? static long size_; static std::vector> chk_conc_, ion_bit_; @@ -414,13 +414,12 @@ void nrn_check_conc_write(Prop* p_ok, Prop* pion, int i) { flag = 0400; } - /* Create a vector holding std::bitset to track which ions + /* Create a vector holding std::bitset to track which ions are being written to the membrane */ if (n_memb_func > size_) { - chk_conc_.resize(2 * n_memb_func); ion_bit_.resize(n_memb_func); - + for (j = size_; j < n_memb_func; ++j) { chk_conc_[2 * j].reset(); chk_conc_[2 * j + 1].reset(); diff --git a/test/hoctests/tests/test_many_ions.py b/test/hoctests/tests/test_many_ions.py index 0f2b4245e6..5ff81b1a8a 100644 --- a/test/hoctests/tests/test_many_ions.py +++ b/test/hoctests/tests/test_many_ions.py @@ -20,9 +20,9 @@ def exists(name): # use up a large number of mechanism indices for ions nion = 1000 -ion_indices = [int(h.ion_register("ca%d" % i, 2)) for i in range(1, nion+1)] +ion_indices = [int(h.ion_register("ca%d" % i, 2)) for i in range(1, nion + 1)] # gain some confidence they exist -assert exists("ca%d_ion"%nion) +assert exists("ca%d_ion" % nion) assert (ion_indices[-1] - ion_indices[0]) == (nion - 1) mt = h.MechanismType(0) assert mt.count() > nion From fc274772dc1b61111ee06aa2ad25d385209eee80 Mon Sep 17 00:00:00 2001 From: wthun <43437733+wthun@users.noreply.github.com> Date: Fri, 20 Sep 2024 13:10:46 +0200 Subject: [PATCH 06/22] rename max_length to max_ion Co-authored-by: Luc Grosheintz --- src/nrnoc/eion.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nrnoc/eion.cpp b/src/nrnoc/eion.cpp index 7a8db092bc..d6f31d8da5 100644 --- a/src/nrnoc/eion.cpp +++ b/src/nrnoc/eion.cpp @@ -401,7 +401,7 @@ The argument `i` specifies which concentration is being written to. It's 0 for exterior; and 1 for interior. */ void nrn_check_conc_write(Prop* p_ok, Prop* pion, int i) { - const int max_length = 10000; // parametrize? + const int max_ions = 64; static long size_; static std::vector> chk_conc_, ion_bit_; From e36452cc24cf61a69b1db17dd04138b456ca8acb Mon Sep 17 00:00:00 2001 From: wthun <43437733+wthun@users.noreply.github.com> Date: Fri, 20 Sep 2024 13:10:57 +0200 Subject: [PATCH 07/22] Update src/nrnoc/eion.cpp Co-authored-by: Luc Grosheintz --- src/nrnoc/eion.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nrnoc/eion.cpp b/src/nrnoc/eion.cpp index d6f31d8da5..d148c2e35a 100644 --- a/src/nrnoc/eion.cpp +++ b/src/nrnoc/eion.cpp @@ -404,7 +404,7 @@ void nrn_check_conc_write(Prop* p_ok, Prop* pion, int i) { const int max_ions = 64; static long size_; - static std::vector> chk_conc_, ion_bit_; + static std::vector> chk_conc_, ion_bit_; Prop* p; int flag, j, k; From 3b5e359fea00ac2b5bbbd52ceed3dc0c9105b9ee Mon Sep 17 00:00:00 2001 From: wthun <43437733+wthun@users.noreply.github.com> Date: Fri, 20 Sep 2024 13:11:08 +0200 Subject: [PATCH 08/22] Update src/nrnoc/eion.cpp Co-authored-by: Luc Grosheintz --- src/nrnoc/eion.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nrnoc/eion.cpp b/src/nrnoc/eion.cpp index d148c2e35a..30195e112a 100644 --- a/src/nrnoc/eion.cpp +++ b/src/nrnoc/eion.cpp @@ -445,7 +445,7 @@ void nrn_check_conc_write(Prop* p_ok, Prop* pion, int i) { continue; } auto rst = chk_conc_[2 * p->_type + i] & ion_bit_[pion->_type]; - if (rst.count() > 0) { + if (rst.any()) { char buf[300]; Sprintf(buf, "%.*s%c is being written at the same location by %s and %s", From dc47da518b1d8b7f81e3ca832d5d9f518a952f14 Mon Sep 17 00:00:00 2001 From: wthun <43437733+wthun@users.noreply.github.com> Date: Fri, 20 Sep 2024 13:11:29 +0200 Subject: [PATCH 09/22] Update src/nrnoc/eion.cpp Co-authored-by: Luc Grosheintz --- src/nrnoc/eion.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nrnoc/eion.cpp b/src/nrnoc/eion.cpp index 30195e112a..224d5f9e21 100644 --- a/src/nrnoc/eion.cpp +++ b/src/nrnoc/eion.cpp @@ -433,7 +433,7 @@ void nrn_check_conc_write(Prop* p_ok, Prop* pion, int i) { if (nrn_is_ion(j)) { ion_bit_[j] = (1 << k); ++k; - assert(k < max_length); + assert(k < max_ions); } } From 570a9aea4d14cbaff7f097da54004a635c081801 Mon Sep 17 00:00:00 2001 From: wthun Date: Fri, 20 Sep 2024 14:10:24 +0200 Subject: [PATCH 10/22] cmake-format --- src/nrnoc/eion.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/nrnoc/eion.cpp b/src/nrnoc/eion.cpp index 224d5f9e21..5a0a921969 100644 --- a/src/nrnoc/eion.cpp +++ b/src/nrnoc/eion.cpp @@ -414,13 +414,12 @@ void nrn_check_conc_write(Prop* p_ok, Prop* pion, int i) { flag = 0400; } - /* Create a vector holding std::bitset to track which ions + /* Create a vector holding std::bitset to track which ions are being written to the membrane */ if (n_memb_func > size_) { - chk_conc_.resize(2 * n_memb_func); ion_bit_.resize(n_memb_func); - + for (j = size_; j < n_memb_func; ++j) { chk_conc_[2 * j].reset(); chk_conc_[2 * j + 1].reset(); From dc11e937becb4f1d0fc17907c8c2d5141548e2ee Mon Sep 17 00:00:00 2001 From: wthun Date: Fri, 20 Sep 2024 14:20:49 +0200 Subject: [PATCH 11/22] increased max_ion to 256 --- src/nrnoc/eion.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nrnoc/eion.cpp b/src/nrnoc/eion.cpp index 5a0a921969..4c841f4c2c 100644 --- a/src/nrnoc/eion.cpp +++ b/src/nrnoc/eion.cpp @@ -401,7 +401,7 @@ The argument `i` specifies which concentration is being written to. It's 0 for exterior; and 1 for interior. */ void nrn_check_conc_write(Prop* p_ok, Prop* pion, int i) { - const int max_ions = 64; + const int max_ions = 256; static long size_; static std::vector> chk_conc_, ion_bit_; From 3f5c418cf222165a28070997f114b46aa93da194 Mon Sep 17 00:00:00 2001 From: Michael Hines Date: Fri, 20 Sep 2024 08:49:24 -0400 Subject: [PATCH 12/22] Temporarily adjust test to create 250 ions. --- test/hoctests/tests/test_many_ions.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/hoctests/tests/test_many_ions.py b/test/hoctests/tests/test_many_ions.py index 5ff81b1a8a..fffd69763e 100644 --- a/test/hoctests/tests/test_many_ions.py +++ b/test/hoctests/tests/test_many_ions.py @@ -18,8 +18,9 @@ def exists(name): return True -# use up a large number of mechanism indices for ions -nion = 1000 +# use up a large number of mechanism indices for ions. And want to test beyond +# 1000 which requires change to max_ions in nrn/src/nrnoc/eion.cpp +nion = 250 ion_indices = [int(h.ion_register("ca%d" % i, 2)) for i in range(1, nion + 1)] # gain some confidence they exist assert exists("ca%d_ion" % nion) From 3f213b090688696763c1c88cd3424a538ad82a77 Mon Sep 17 00:00:00 2001 From: Michael Hines Date: Fri, 20 Sep 2024 11:20:59 -0400 Subject: [PATCH 13/22] Use generic mod file loader --- test/hoctests/tests/test_many_ions.py | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/test/hoctests/tests/test_many_ions.py b/test/hoctests/tests/test_many_ions.py index fffd69763e..c08eb55941 100644 --- a/test/hoctests/tests/test_many_ions.py +++ b/test/hoctests/tests/test_many_ions.py @@ -4,7 +4,7 @@ # the neurondemo mod file shared library which will create the ca ion # along with several mechanisms that write to cai. -from neuron import h +from neuron import h, load_mechanisms from platform import machine import sys @@ -33,14 +33,11 @@ def exists(name): # load neurondemo mod files. That will create ca_ion and provide two # mod files that write cai -nrnmechlib = ( - "nrnmech.dll" if sys.platform == "win32" else "%s/libnrnmech.so" % machine() -) # Following Aborts prior to PR#3055 with # eion.cpp:431: void nrn_check_conc_write(Prop*, Prop*, int): Assertion `k < sizeof(long) * 8' failed. -nrnmechlibname = "%s/demo/release/%s" % (h.neuronhome(), nrnmechlib) -print(nrnmechlibname) -assert h.nrn_load_dll(nrnmechlibname) +nrnmechlibpath = "%s/demo/release" % h.neuronhome() +print(nrnmechlibpath) +assert load_mechanisms(nrnmechlibpath) # ca_ion now exists and has a mechanism index > nion assert exists("ca_ion") From 23071ee294a6285eedb9fe37e4da8ec8513befda Mon Sep 17 00:00:00 2001 From: Michael Hines Date: Fri, 20 Sep 2024 16:24:30 -0400 Subject: [PATCH 14/22] ion_bit_ can have many bits. --- src/nrnoc/eion.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/nrnoc/eion.cpp b/src/nrnoc/eion.cpp index 4c841f4c2c..ac6ee57f06 100644 --- a/src/nrnoc/eion.cpp +++ b/src/nrnoc/eion.cpp @@ -430,9 +430,9 @@ void nrn_check_conc_write(Prop* p_ok, Prop* pion, int i) { } for (k = 0, j = 0; j < n_memb_func; ++j) { if (nrn_is_ion(j)) { - ion_bit_[j] = (1 << k); - ++k; assert(k < max_ions); + ion_bit_[j].set(k); + ++k; } } From 6604aab20c15efc26ffcaf26dbb9e8739e85a3cf Mon Sep 17 00:00:00 2001 From: Michael Hines Date: Fri, 20 Sep 2024 19:38:45 -0400 Subject: [PATCH 15/22] For bitset single bit, first reset, then set the bit. --- src/nrnoc/eion.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/nrnoc/eion.cpp b/src/nrnoc/eion.cpp index ac6ee57f06..71393a2bb0 100644 --- a/src/nrnoc/eion.cpp +++ b/src/nrnoc/eion.cpp @@ -431,6 +431,7 @@ void nrn_check_conc_write(Prop* p_ok, Prop* pion, int i) { for (k = 0, j = 0; j < n_memb_func; ++j) { if (nrn_is_ion(j)) { assert(k < max_ions); + ion_bit_[j].reset(); ion_bit_[j].set(k); ++k; } From 0d087b743324bff7f7ad8a8fe0b01e3cdb07a0ba Mon Sep 17 00:00:00 2001 From: Michael Hines Date: Sat, 21 Sep 2024 09:34:46 -0400 Subject: [PATCH 16/22] Add another mod file to neurondemo that WRITE cai. More testing. --- share/demo/release/capmpr.mod | 90 +++++++++++++++++++++++++++ test/hoctests/tests/test_many_ions.py | 44 +++++++++++++ 2 files changed, 134 insertions(+) create mode 100644 share/demo/release/capmpr.mod diff --git a/share/demo/release/capmpr.mod b/share/demo/release/capmpr.mod new file mode 100644 index 0000000000..40abd43d13 --- /dev/null +++ b/share/demo/release/capmpr.mod @@ -0,0 +1,90 @@ +: capump.mod plus a "reservoir" used to initialize cai to desired concentrations + +UNITS { + (mM) = (milli/liter) + (mA) = (milliamp) + (um) = (micron) + (mol) = (1) + PI = (pi) (1) + FARADAY = (faraday) (coulomb) +} + +NEURON { + SUFFIX capmpr + USEION ca READ cao, cai WRITE cai, ica + GLOBAL k1, k2, k3, k4 + GLOBAL car, tau +} + +STATE { + pump (mol/cm2) + pumpca (mol/cm2) + cai (mM) +} + +PARAMETER { + car = 5e-5 (mM) : ca in reservoir, used to initialize cai to desired concentrations + tau = 1e9 (ms) : rate of equilibration between cai and car + + k1 = 5e8 (/mM-s) + k2 = .25e6 (/s) + k3 = .5e3 (/s) + k4 = 5e0 (/mM-s) + + pumpdens = 3e-14 (mol/cm2) +} + +CONSTANT { + volo = 1 (liter) +} + +ASSIGNED { + diam (um) + cao (mM) + + ica (mA/cm2) + ipump (mA/cm2) + ipump_last (mA/cm2) + voli (um3) + area1 (um2) + c1 (1+8 um5/ms) + c2 (1-10 um2/ms) + c3 (1-10 um2/ms) + c4 (1+8 um5/ms) +} + +BREAKPOINT { + SOLVE pmp METHOD sparse + ipump_last = ipump + ica = ipump +} + +KINETIC pmp { + COMPARTMENT voli {cai} + COMPARTMENT (1e10)*area1 {pump pumpca} + COMPARTMENT volo*(1e15) {cao car} + + ~ car <-> cai (1(um3)/tau,1(um3)/tau) + ~ cai + pump <-> pumpca (c1,c2) + ~ pumpca <-> pump + cao (c3,c4) + + : note that forward flux here is the outward flux + ipump = (1e-4)*2*FARADAY*(f_flux - b_flux)/area1 + + : ipump_last vs ipump needed because of STEADYSTATE calculation + ~ cai << (-(ica - ipump_last)*area1/(2*FARADAY)*(1e4)) + + CONSERVE pump + pumpca = (1e10)*area1*pumpdens +} + +INITIAL { + :cylindrical coordinates; actually vol and area1/unit length + voli = PI*(diam/2)^2 * 1(um) + area1 = 2*PI*(diam/2) * 1(um) + c1 = (1e7)*area1 * k1 + c2 = (1e7)*area1 * k2 + c3 = (1e7)*area1 * k3 + c4 = (1e7)*area1 * k4 + + SOLVE pmp STEADYSTATE sparse +} diff --git a/test/hoctests/tests/test_many_ions.py b/test/hoctests/tests/test_many_ions.py index c08eb55941..05bff06cb5 100644 --- a/test/hoctests/tests/test_many_ions.py +++ b/test/hoctests/tests/test_many_ions.py @@ -7,6 +7,7 @@ from neuron import h, load_mechanisms from platform import machine import sys +import io # return True if name exists in h @@ -45,7 +46,50 @@ def exists(name): mt.select("ca_ion") assert mt.selected() > nion + +# return stderr output resulting from sec.insert(mechname) +def mech_insert(sec, mechname): + # capture stderr + old_stderr = sys.stderr + sys.stderr = mystderr = io.StringIO() + + sec.insert(mechname) + + sys.stderr = old_stderr + return mystderr.getvalue() + + # test if can use the high mechanism index cadifpmp (with USEION ... WRITE cai) along with the high mechanims index ca_ion s = h.Section() s.insert("cadifpmp") h.finitialize(-65) +# The ion_style tells whether cai or cao is being written +istyle = int(h.ion_style("ca_ion", sec=s)) +assert istyle & 0o200 # writing cai +assert (istyle & 0o400) == 0 # not writing ca0 +# unfortunately, at present, uninserting does not turn off that bit +s.uninsert("cadifpmp") +assert s.has_membrane("cadifpmp") is False +assert int(h.ion_style("ca_ion", sec=s)) & 0o200 +# nevertheless, on reinsertion, there is no warning, so can't test the +# warning without a second mechanism that WRITE cai +assert mech_insert(s, "cadifpmp") == "" +assert s.has_membrane("cadifpmp") +# uninsert again and insert another mechanism that writes. +s.uninsert("cadifpmp") +assert mech_insert(s, "capmpr") == "" # no warning +assert mech_insert(s, "cadifpmp") != "" # now there is a warning. + +# more serious test +# several sections with alternating cadifpmp and capmpr +del s +secs = [h.Section() for i in range(5)] +for i, s in enumerate(secs): + if i % 2: + assert mech_insert(s, "capmpr") == "" + else: + assert mech_insert(s, "cadifpmp") == "" + +# warnings due to multiple mechanisms at same location that WRITE cai +assert mech_insert(secs[2], "capmpr") != "" +assert mech_insert(secs[3], "cadifpmp") != "" From bfc82da1c92bd3c5cb34319f2b4540513fc04909 Mon Sep 17 00:00:00 2001 From: Michael Hines Date: Sat, 21 Sep 2024 11:10:08 -0400 Subject: [PATCH 17/22] eion.cpp -Wall free --- src/nrnoc/eion.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nrnoc/eion.cpp b/src/nrnoc/eion.cpp index 71393a2bb0..e8e7657b7e 100644 --- a/src/nrnoc/eion.cpp +++ b/src/nrnoc/eion.cpp @@ -641,7 +641,7 @@ void second_order_cur(NrnThread* nt) { extern int secondorder; NrnThreadMembList* tml; Memb_list* ml; - int j, i, i2; + int i, i2; constexpr auto c = 3; constexpr auto dc = 4; if (secondorder == 2) { From ebf26b1987262ff949397a525d346c9fb1b51d70 Mon Sep 17 00:00:00 2001 From: Michael Hines Date: Sat, 21 Sep 2024 20:07:55 -0400 Subject: [PATCH 18/22] eion.cpp coverage 100% functions 94% lines --- test/hoctests/tests/test_eion_cover.py | 62 ++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 test/hoctests/tests/test_eion_cover.py diff --git a/test/hoctests/tests/test_eion_cover.py b/test/hoctests/tests/test_eion_cover.py new file mode 100644 index 0000000000..73e96aa35c --- /dev/null +++ b/test/hoctests/tests/test_eion_cover.py @@ -0,0 +1,62 @@ +from neuron import h +from neuron.expect_hocerr import expect_err, set_quiet + +set_quiet(0) +from math import isclose + + +def test_nernst(): + assert h.nernst(1, 1, 0) == 0.0 + assert h.nernst(-1, 1, 1) == 1e6 + assert h.nernst(1, -1, 1) == -1e6 + + s = h.Section() + s.insert("hh") + assert isclose(h.nernst("ena", sec=s), 63.55150321937486) + assert isclose(h.nernst("ena", 0.5, sec=s), 63.55150321937486) + assert isclose(h.nernst("nai", sec=s), 17.554820246530547) + assert isclose(h.nernst("nao", sec=s), 79.7501757545304) + expect_err('h.nernst("ina", sec=s)') + + del s + locals() + + +def test_ghk(): + assert isclose(h.ghk(-10, 0.1, 10, 2), -2828.3285716150644) + assert isclose(h.ghk(1e-6, 0.1, 10, 2), -1910.40949510667) + + +def test_ion_style(): + expect_err('h.ion_style("foo")') + + +def test_second_order_cur(): + s = h.Section() + s.insert("hh") + h.secondorder = 2 + h.finitialize(-65) + h.fadvance() + assert isclose(s(0.5).ina, -0.001220053188847315) + h.secondorder = 0 + h.finitialize(-65) + h.fadvance() + assert isclose(s(0.5).ina, -0.0012200571764654333) + + +def test_ion_charge(): + assert h.ion_charge("na_ion") == 1 + expect_err('h.ion_charge("na") == 1') + + +def test_eion_cover(): + test_nernst() + test_ghk() + test_ion_style() + test_second_order_cur() + test_ion_charge() + + +if __name__ == "__main__": + test_eion_cover() + h.topology() From 06f9acec59f96de18258cde31f1b0107acef802d Mon Sep 17 00:00:00 2001 From: Michael Hines Date: Sun, 22 Sep 2024 08:10:47 -0400 Subject: [PATCH 19/22] Replace // LCOV_EXCL_END with // LCOV_EXCL_STOP --- src/ivoc/apwindow.cpp | 2 +- src/ivoc/pwman.cpp | 2 +- src/sundials/cvodes/cvodes.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ivoc/apwindow.cpp b/src/ivoc/apwindow.cpp index 71c07bcf36..e5afea2fc2 100644 --- a/src/ivoc/apwindow.cpp +++ b/src/ivoc/apwindow.cpp @@ -415,7 +415,7 @@ bool PrintableWindow::receive(const Event& e) { reconfigured(); notify(); break; - // LCOV_EXCL_END + // LCOV_EXCL_STOP case MapNotify: if (xplace_) { if (xtop() != xtop_ || xleft() != xleft_) { diff --git a/src/ivoc/pwman.cpp b/src/ivoc/pwman.cpp index 8489000cb6..55e1fa1a1f 100644 --- a/src/ivoc/pwman.cpp +++ b/src/ivoc/pwman.cpp @@ -1477,7 +1477,7 @@ void PrintableWindow::reconfigured() { xmove(x, y); } } -// LCOV_EXCL_END +// LCOV_EXCL_STOP void ViewWindow::reconfigured() { if (!pixres) { diff --git a/src/sundials/cvodes/cvodes.c b/src/sundials/cvodes/cvodes.c index 8fee9feabb..48a96db636 100755 --- a/src/sundials/cvodes/cvodes.c +++ b/src/sundials/cvodes/cvodes.c @@ -3285,7 +3285,7 @@ static int CVStep(CVodeMem cv_mem) dsm = CVStgrUpdateDsm(cv_mem, dsm, dsmS); } } - // LCOV_EXCL_END + // LCOV_EXCL_STOP /* Everything went fine; exit loop */ break; From aea62999e483d8b8dfdc078675f799885f1d0992 Mon Sep 17 00:00:00 2001 From: Michael Hines Date: Mon, 23 Sep 2024 16:14:03 -0400 Subject: [PATCH 20/22] no limit on number of ion mechanisms --- src/nrnoc/eion.cpp | 80 +++++++++++++++++++++++----------------------- 1 file changed, 40 insertions(+), 40 deletions(-) diff --git a/src/nrnoc/eion.cpp b/src/nrnoc/eion.cpp index e8e7657b7e..6dece3efb8 100644 --- a/src/nrnoc/eion.cpp +++ b/src/nrnoc/eion.cpp @@ -15,9 +15,8 @@ #include #include -#include -#include -#include +#include +#include #undef hoc_retpushx @@ -394,17 +393,37 @@ double nrn_nernst_coef(int type) { /* It is generally an error for two models to WRITE the same concentration. -This functions checks that there's no write conflict; and warns if it detects -one. It also sets respective write flag in the style of the ion. +nrn_check_conc_write checks that there's no write conflict; and warns if it +detects one. It also sets respective write flag in the style of the ion. The argument `i` specifies which concentration is being written to. It's 0 for exterior; and 1 for interior. */ -void nrn_check_conc_write(Prop* p_ok, Prop* pion, int i) { - const int max_ions = 256; - static long size_; - static std::vector> chk_conc_, ion_bit_; +// Each mechanism type index that writes a concentration has a set of ion +// type indices it writes to. +// ionconctype is coded as iontype*2 + i where i=1 for interior +// So number of distinct ion mechanisms is essentially unlimited, max_int/2. +static std::map> mechtype2ionconctype; + +static void add_mechtype2ionconctype(int mechtype, int iontype, int i) { + auto& set_of_ionconctypes = mechtype2ionconctype[mechtype]; + set_of_ionconctypes.insert(2 * iontype + i); // unique though inserting for each instance. +} + +static bool mech_uses_ionconctype(int mechtype, int iontype, int i) { + if (auto search = mechtype2ionconctype.find(mechtype); search != mechtype2ionconctype.end()) { + auto& set_of_ionconctypes = search->second; + return set_of_ionconctypes.count(2 * iontype + i) == 1; + } + return false; +} + +void nrn_check_conc_write(Prop* pmech, Prop* pion, int i) { + // Would be less redundant to generate the "database" at + // mechanism registration time. But NMODL presently gives us this info + // only at each mechanism instance allocation. + add_mechtype2ionconctype(pmech->_type, pion->_type, i); Prop* p; int flag, j, k; @@ -414,51 +433,32 @@ void nrn_check_conc_write(Prop* p_ok, Prop* pion, int i) { flag = 0400; } - /* Create a vector holding std::bitset to track which ions - are being written to the membrane */ - if (n_memb_func > size_) { - chk_conc_.resize(2 * n_memb_func); - ion_bit_.resize(n_memb_func); - - for (j = size_; j < n_memb_func; ++j) { - chk_conc_[2 * j].reset(); - chk_conc_[2 * j + 1].reset(); - ion_bit_[j].reset(); - } - - size_ = n_memb_func; - } - for (k = 0, j = 0; j < n_memb_func; ++j) { - if (nrn_is_ion(j)) { - assert(k < max_ions); - ion_bit_[j].reset(); - ion_bit_[j].set(k); - ++k; - } - } - - chk_conc_[2 * p_ok->_type + i] |= ion_bit_[pion->_type]; - if (pion->dparam[iontype_index_dparam].get() & flag) { - /* now comes the hard part. Is the possibility in fact actual.*/ + auto ii = pion->dparam[iontype_index_dparam].get(); + if (ii & flag) { + // Is the possibility in fact actual. Unfortunately, uninserting the + // mechanism that writes a concentration does not reset the flag bit. + // So search the node property list for another mechanism that also + // writes this ion concentration. (that is needed anyway to + // fill out the warning message.) + + // the pion in the node property list is before mechanisms that use the ion for (p = pion->next; p; p = p->next) { - if (p == p_ok) { + if (p == pmech) { continue; } - auto rst = chk_conc_[2 * p->_type + i] & ion_bit_[pion->_type]; - if (rst.any()) { + if (mech_uses_ionconctype(p->_type, pion->_type, i)) { char buf[300]; Sprintf(buf, "%.*s%c is being written at the same location by %s and %s", (int) strlen(memb_func[pion->_type].sym->name) - 4, memb_func[pion->_type].sym->name, ((i == 1) ? 'i' : 'o'), - memb_func[p_ok->_type].sym->name, + memb_func[pmech->_type].sym->name, memb_func[p->_type].sym->name); hoc_warning(buf, (char*) 0); } } } - auto ii = pion->dparam[iontype_index_dparam].get(); ii |= flag; pion->dparam[iontype_index_dparam] = ii; } From aceb0c562dedb050f6a1517b90c19a0dc0cb83b3 Mon Sep 17 00:00:00 2001 From: Michael Hines Date: Thu, 26 Sep 2024 15:47:26 -0400 Subject: [PATCH 21/22] Code change for ion and ion style semantics. Unlimited numbers. --- cmake/NeuronFileLists.cmake | 1 + src/coreneuron/io/nrn_checkpoint.cpp | 6 ++++-- src/coreneuron/io/phase2.cpp | 5 +++-- src/coreneuron/mechanism/register_mech.cpp | 9 +++++---- src/coreneuron/permute/node_permute.cpp | 5 +++-- src/neuron/cache/mechanism_range.hpp | 2 +- .../nrncore_write/callbacks/nrncore_callbacks.cpp | 6 +++--- src/nrniv/nrncore_write/data/cell_group.cpp | 6 +++--- src/nrniv/nrncore_write/data/cell_group.h | 4 ++-- src/nrnoc/init.cpp | 4 ++-- src/nrnoc/ion_semantics.h | 14 ++++++++++++++ src/nrnoc/membfunc.h | 1 + 12 files changed, 42 insertions(+), 21 deletions(-) create mode 100644 src/nrnoc/ion_semantics.h diff --git a/cmake/NeuronFileLists.cmake b/cmake/NeuronFileLists.cmake index 9809bf8f88..283226c677 100644 --- a/cmake/NeuronFileLists.cmake +++ b/cmake/NeuronFileLists.cmake @@ -21,6 +21,7 @@ set(HEADER_FILES_TO_INSTALL nrnoc/md1redef.h nrnoc/md2redef.h nrnoc/membdef.h + nrnoc/ion_semantics.h nrnoc/membfunc.h nrnoc/multicore.h nrnoc/multisplit.h diff --git a/src/coreneuron/io/nrn_checkpoint.cpp b/src/coreneuron/io/nrn_checkpoint.cpp index ed91d7de81..c7c7743ed9 100644 --- a/src/coreneuron/io/nrn_checkpoint.cpp +++ b/src/coreneuron/io/nrn_checkpoint.cpp @@ -11,6 +11,7 @@ #include #include +#include "nrnoc/ion_semantics.h" #include "coreneuron/sim/multicore.hpp" #include "coreneuron/nrniv/nrniv_decl.h" #include "coreneuron/io/nrn_filehandler.hpp" @@ -296,8 +297,9 @@ void CheckPoints::write_phase2(NrnThread& nt) const { // out into the following function. d[ix] = nrn_original_aos_index(ptype, d[ix], nt, ml_pinv); } - } else if (s >= 0 && s < 1000) { // ion - d[ix] = nrn_original_aos_index(s, d[ix], nt, ml_pinv); + } else if (nrn_semantics_is_ion(s)) { // ion + auto type = nrn_semantics_ion_type(s); + d[ix] = nrn_original_aos_index(type, d[ix], nt, ml_pinv); } #if CHKPNTDEBUG if (s != -8) { // WATCH values change diff --git a/src/coreneuron/io/phase2.cpp b/src/coreneuron/io/phase2.cpp index 19fb12e8a3..3c712f1f5b 100644 --- a/src/coreneuron/io/phase2.cpp +++ b/src/coreneuron/io/phase2.cpp @@ -6,6 +6,7 @@ # ============================================================================= */ +#include "nrnoc/ion_semantics.h" #include "coreneuron/io/phase2.hpp" #include "coreneuron/coreneuron.hpp" #include "coreneuron/sim/multicore.hpp" @@ -699,8 +700,8 @@ void Phase2::pdata_relocation(const NrnThread& nt, const std::vector& **/ break; default: - if (s >= 0 && s < 1000) { // ion - int etype = s; + if (nrn_semantics_is_ion(s)) { // ion + int etype = nrn_semantics_ion_type(s); /* if ion is SoA, must recalculate pdata values */ /* if ion is AoS, have to deal with offset */ Memb_list* eml = nt._ml_list[etype]; diff --git a/src/coreneuron/mechanism/register_mech.cpp b/src/coreneuron/mechanism/register_mech.cpp index 260d420411..2ef95e976f 100644 --- a/src/coreneuron/mechanism/register_mech.cpp +++ b/src/coreneuron/mechanism/register_mech.cpp @@ -9,6 +9,7 @@ #include #include "coreneuron/nrnconf.h" +#include "nrnoc/ion_semantics.h" #include "coreneuron/sim/multicore.hpp" #include "coreneuron/membrane_definitions.h" #include "coreneuron/mechanism/eion.hpp" @@ -212,7 +213,7 @@ void hoc_register_dparam_semantics(int type, int ix, const char* name) { xx_ion and #xx_ion which will get a semantics value of -1, -2, -3, -4, -5, -6, -7, -8, -9, -10, - type, and type+1000 respectively + 2*type, and 2*type+1 respectively */ auto& memb_func = corenrn.get_memb_funcs(); if (strcmp(name, "area") == 0) { @@ -240,7 +241,7 @@ void hoc_register_dparam_semantics(int type, int ix, const char* name) { } else { int i = name[0] == '#' ? 1 : 0; int etype = nrn_get_mechtype(name + i); - memb_func[type].dparam_semantics[ix] = etype + i * 1000; + memb_func[type].dparam_semantics[ix] = nrn_semantics_from_ion(etype, i); /* note that if style is needed (i==1), then we are writing a concentration */ if (i) { ion_write_depend(type, etype); @@ -300,8 +301,8 @@ int nrn_mech_depend(int type, int* dependencies) { int idep = 0; if (ds) for (int i = 0; i < dpsize; ++i) { - if (ds[i] > 0 && ds[i] < 1000) { - int deptype = ds[i]; + if (nrn_semantics_is_ion(ds[i])) { + int deptype = nrn_semantics_ion_type(ds[i]); int idepnew = depend_append(idep, dependencies, deptype, type); if ((idepnew > idep) && !corenrn.get_ion_write_dependency().empty() && !corenrn.get_ion_write_dependency()[deptype].empty()) { diff --git a/src/coreneuron/permute/node_permute.cpp b/src/coreneuron/permute/node_permute.cpp index a19230b4fc..eb33da0991 100644 --- a/src/coreneuron/permute/node_permute.cpp +++ b/src/coreneuron/permute/node_permute.cpp @@ -94,6 +94,7 @@ so pdata_m(k, isz) = inew + data_t #include "coreneuron/nrniv/nrniv_decl.h" #include "coreneuron/utils/nrn_assert.h" #include "coreneuron/coreneuron.hpp" +#include "nrnoc/ion_semantics.h" #else #include "nrnoc/multicore.h" #include "oc/nrnassrt.h" @@ -333,8 +334,8 @@ static void update_pdata_values(Memb_list* ml, int type, NrnThread& nt) { nrn_assert(0); } } - } else if (s >= 0 && s < 1000) { // ion - int etype = s; + } else if (nrn_semantics_is_ion(s)) { // ion + int etype = nrn_semantics_ion_type(s); int elayout = corenrn.get_mech_data_layout()[etype]; Memb_list* eml = nt._ml_list[etype]; int edata0 = eml->data - nt._data; diff --git a/src/neuron/cache/mechanism_range.hpp b/src/neuron/cache/mechanism_range.hpp index 4a332ebb06..8cca0ce44d 100644 --- a/src/neuron/cache/mechanism_range.hpp +++ b/src/neuron/cache/mechanism_range.hpp @@ -22,7 +22,7 @@ void indices_to_cache(short type, Callable callable) { auto const sem = dparam_semantics[field]; // See https://github.com/neuronsimulator/nrn/issues/2312 for discussion of possible // extensions to caching. - if ((sem > 0 && sem < 1000) || sem == -1 /* area */ || sem == -9 /* diam */) { + if (nrn_semantics_is_ion(sem) || sem == -1 /* area */ || sem == -9 /* diam */) { std::invoke(callable, field); } } diff --git a/src/nrniv/nrncore_write/callbacks/nrncore_callbacks.cpp b/src/nrniv/nrncore_write/callbacks/nrncore_callbacks.cpp index 158ae0c028..a91b69ec14 100644 --- a/src/nrniv/nrncore_write/callbacks/nrncore_callbacks.cpp +++ b/src/nrniv/nrncore_write/callbacks/nrncore_callbacks.cpp @@ -670,10 +670,10 @@ int* datum2int(int type, } } else if (etype == -9) { pdata[jj] = eindex; - } else if (etype > 0 && etype < 1000) { // ion pointer + } else if (nrn_semantics_is_ion(etype)) { // ion pointer pdata[jj] = eindex; - } else if (etype > 1000 && etype < 2000) { // ionstyle can be explicit instead of - // pointer to int* + } else if (nrn_semantics_is_ionstyle(etype)) { + // ionstyle can be explicit instead of pointer to int* pdata[jj] = eindex; } else if (etype == -2) { // an ion and this is the iontype pdata[jj] = eindex; diff --git a/src/nrniv/nrncore_write/data/cell_group.cpp b/src/nrniv/nrncore_write/data/cell_group.cpp index e034359387..8ee75d4191 100644 --- a/src/nrniv/nrncore_write/data/cell_group.cpp +++ b/src/nrniv/nrncore_write/data/cell_group.cpp @@ -343,15 +343,15 @@ void CellGroup::datumindex_fill(int ith, CellGroup& cg, DatumIndices& di, Memb_l } assert(etype != 0); // pointer into one of the tml types? - } else if (dmap[j] > 0 && dmap[j] < 1000) { // double* into eion type data - etype = dmap[j]; + } else if (nrn_semantics_is_ion(dmap[j])) { // double* into eion type data + etype = nrn_semantics_ion_type(dmap[j]); Memb_list* eml = cg.type2ml[etype]; assert(eml); auto* const pval = dparam[j].get(); auto const legacy_index = eml->legacy_index(pval); assert(legacy_index >= 0); eindex = legacy_index; - } else if (dmap[j] > 1000) { // int* into ion dparam[xxx][0] + } else if (nrn_semantics_is_ionstyle(dmap[j])) { // int* into ion dparam[xxx][0] // store the actual ionstyle etype = dmap[j]; eindex = *dparam[j].get(); diff --git a/src/nrniv/nrncore_write/data/cell_group.h b/src/nrniv/nrncore_write/data/cell_group.h index 374c2a2510..e86b1500cb 100644 --- a/src/nrniv/nrncore_write/data/cell_group.h +++ b/src/nrniv/nrncore_write/data/cell_group.h @@ -32,8 +32,8 @@ class CellGroup { // following three are parallel arrays std::vector output_ps; // n_presyn of these, real are first, tml order for acell. std::vector output_gid; // n_presyn of these, -(type + 1000*index) if no gid - std::vector output_vindex; // n_presyn of these. >=0 if associated with voltage, -(type + - // 1000*index) for acell. + std::vector output_vindex; // n_presyn of these. >=0 if associated with voltage, + // -(type + 1000*index) for acell. int n_netcon; // all that have targets associated with this threads Point_process. std::vector netcons; int* netcon_srcgid; // -(type + 1000*index) refers to acell with no gid diff --git a/src/nrnoc/init.cpp b/src/nrnoc/init.cpp index f233507f70..a41f95937b 100644 --- a/src/nrnoc/init.cpp +++ b/src/nrnoc/init.cpp @@ -765,7 +765,7 @@ namespace { */ // name to int map for the negative types -// xx_ion and #xx_ion will get values of type and type+1000 respectively +// xx_ion and #xx_ion will get values of type*2 and type*2+1 respectively static std::unordered_map name_to_negint = {{"area", -1}, {"iontype", -2}, {"cvodeieq", -3}, @@ -785,7 +785,7 @@ int dparam_semantics_to_int(std::string_view name) { bool const i{name[0] == '#'}; Symbol* s = hoc_lookup(std::string{name.substr(i)}.c_str()); if (s && s->type == MECHANISM) { - return s->subtype + i * 1000; + return s->subtype * 2 + i; } throw std::runtime_error("unknown dparam semantics: " + std::string{name}); } diff --git a/src/nrnoc/ion_semantics.h b/src/nrnoc/ion_semantics.h new file mode 100644 index 0000000000..2a3dfc8253 --- /dev/null +++ b/src/nrnoc/ion_semantics.h @@ -0,0 +1,14 @@ +#pragma once + +inline int nrn_semantics_from_ion(int type, int i) { + return 2 * type + i; +} +inline bool nrn_semantics_is_ion(int i) { + return i >= 0 && (i & 1) == 0; +} +inline bool nrn_semantics_is_ionstyle(int i) { + return i >= 0 && (i & 1) == 1; +} +inline int nrn_semantics_ion_type(int i) { + return i / 2; +} diff --git a/src/nrnoc/membfunc.h b/src/nrnoc/membfunc.h index ad80ed9d7e..18647c19b7 100644 --- a/src/nrnoc/membfunc.h +++ b/src/nrnoc/membfunc.h @@ -5,6 +5,7 @@ extern void hoc_register_prop_size(int type, int psize, int dpsize); #include "nrnoc_ml.h" #include "oc_ansi.h" // neuron::model_sorted_token #include "options.h" // EXTRACELLULAR +#include "ion_semantics.h" #include #include From 1318a3f13ddbd496c7a0ecf528fd5d679372ca6d Mon Sep 17 00:00:00 2001 From: Michael Hines Date: Mon, 30 Sep 2024 09:03:13 -0400 Subject: [PATCH 22/22] Fix the CI test coverage failure. --- test/hoctests/tests/test_many_ions.py | 95 --------------------- test/hoctests/tests/test_neurondemo.py | 113 +++++++++++++++++++++++++ 2 files changed, 113 insertions(+), 95 deletions(-) delete mode 100644 test/hoctests/tests/test_many_ions.py diff --git a/test/hoctests/tests/test_many_ions.py b/test/hoctests/tests/test_many_ions.py deleted file mode 100644 index 05bff06cb5..0000000000 --- a/test/hoctests/tests/test_many_ions.py +++ /dev/null @@ -1,95 +0,0 @@ -# Test when large number of ion mechanisms exist. - -# Strategy is to create a large number of ion mechanisms before loading -# the neurondemo mod file shared library which will create the ca ion -# along with several mechanisms that write to cai. - -from neuron import h, load_mechanisms -from platform import machine -import sys -import io - - -# return True if name exists in h -def exists(name): - try: - exec("h.%s" % name) - except: - return False - return True - - -# use up a large number of mechanism indices for ions. And want to test beyond -# 1000 which requires change to max_ions in nrn/src/nrnoc/eion.cpp -nion = 250 -ion_indices = [int(h.ion_register("ca%d" % i, 2)) for i in range(1, nion + 1)] -# gain some confidence they exist -assert exists("ca%d_ion" % nion) -assert (ion_indices[-1] - ion_indices[0]) == (nion - 1) -mt = h.MechanismType(0) -assert mt.count() > nion - -# this test depends on the ca ion not existing at this point -assert exists("ca_ion") is False - -# load neurondemo mod files. That will create ca_ion and provide two -# mod files that write cai -# Following Aborts prior to PR#3055 with -# eion.cpp:431: void nrn_check_conc_write(Prop*, Prop*, int): Assertion `k < sizeof(long) * 8' failed. -nrnmechlibpath = "%s/demo/release" % h.neuronhome() -print(nrnmechlibpath) -assert load_mechanisms(nrnmechlibpath) - -# ca_ion now exists and has a mechanism index > nion -assert exists("ca_ion") -mt = h.MechanismType(0) # new mechanisms do not appear in old MechanismType -mt.select("ca_ion") -assert mt.selected() > nion - - -# return stderr output resulting from sec.insert(mechname) -def mech_insert(sec, mechname): - # capture stderr - old_stderr = sys.stderr - sys.stderr = mystderr = io.StringIO() - - sec.insert(mechname) - - sys.stderr = old_stderr - return mystderr.getvalue() - - -# test if can use the high mechanism index cadifpmp (with USEION ... WRITE cai) along with the high mechanims index ca_ion -s = h.Section() -s.insert("cadifpmp") -h.finitialize(-65) -# The ion_style tells whether cai or cao is being written -istyle = int(h.ion_style("ca_ion", sec=s)) -assert istyle & 0o200 # writing cai -assert (istyle & 0o400) == 0 # not writing ca0 -# unfortunately, at present, uninserting does not turn off that bit -s.uninsert("cadifpmp") -assert s.has_membrane("cadifpmp") is False -assert int(h.ion_style("ca_ion", sec=s)) & 0o200 -# nevertheless, on reinsertion, there is no warning, so can't test the -# warning without a second mechanism that WRITE cai -assert mech_insert(s, "cadifpmp") == "" -assert s.has_membrane("cadifpmp") -# uninsert again and insert another mechanism that writes. -s.uninsert("cadifpmp") -assert mech_insert(s, "capmpr") == "" # no warning -assert mech_insert(s, "cadifpmp") != "" # now there is a warning. - -# more serious test -# several sections with alternating cadifpmp and capmpr -del s -secs = [h.Section() for i in range(5)] -for i, s in enumerate(secs): - if i % 2: - assert mech_insert(s, "capmpr") == "" - else: - assert mech_insert(s, "cadifpmp") == "" - -# warnings due to multiple mechanisms at same location that WRITE cai -assert mech_insert(secs[2], "capmpr") != "" -assert mech_insert(secs[3], "cadifpmp") != "" diff --git a/test/hoctests/tests/test_neurondemo.py b/test/hoctests/tests/test_neurondemo.py index b3c8d6bcbf..78f88f861e 100644 --- a/test/hoctests/tests/test_neurondemo.py +++ b/test/hoctests/tests/test_neurondemo.py @@ -13,6 +13,7 @@ dir_path = os.path.dirname(os.path.realpath(__file__)) chk = Chk(os.path.join(dir_path, "test_neurondemo.json")) + # Run a command with input into stdin def run(cmd, input): p = Popen(cmd, shell=True, stdin=PIPE, stdout=PIPE, stderr=PIPE, text=True) @@ -47,6 +48,7 @@ def run(cmd, input): quit() """ + # run neurondemo and return the stdout lines between ZZZbegin and ZZZend def neurondemo(extra, input): input = extra + input @@ -172,3 +174,114 @@ def neurondemo(extra, input): chk(key, rich_data) chk.save() + +################################################ +# test_many_ions.py copied below and removed to fix a CI coverage test failure: +# libgcov profiling error:/home/...demo/release/x86_64/mod_func.gcda:overwriting +# an existing profile data with a different timestamp. +# I was unable to reproduce the coverage test failure on my linux desktop by +# experimenting with parallel runs of the distinct tests. Nevertheless, +# CI coverage is successful when the use of the neurondemo shared library is +# serialized into this file. +# I'd rather keep the file and run from here via something like +# run("python test_many_ions.py", "") +# but cmake installs each hoctest file in a separate folder and "python" may not +# be the name of the program we need to run. +########################### +# Test when large number of ion mechanisms exist. + +# Strategy is to create a large number of ion mechanisms before loading +# the neurondemo mod file shared library which will create the ca ion +# along with several mechanisms that write to cai. + +from neuron import h, load_mechanisms +from platform import machine +import sys +import io + + +# return True if name exists in h +def exists(name): + try: + exec("h.%s" % name) + except: + return False + return True + + +# use up a large number of mechanism indices for ions. And want to test beyond +# 1000 which requires change to max_ions in nrn/src/nrnoc/eion.cpp +nion = 250 +ion_indices = [int(h.ion_register("ca%d" % i, 2)) for i in range(1, nion + 1)] +# gain some confidence they exist +assert exists("ca%d_ion" % nion) +assert (ion_indices[-1] - ion_indices[0]) == (nion - 1) +mt = h.MechanismType(0) +assert mt.count() > nion + +# this test depends on the ca ion not existing at this point +assert exists("ca_ion") is False + +# load neurondemo mod files. That will create ca_ion and provide two +# mod files that write cai +# Following Aborts prior to PR#3055 with +# eion.cpp:431: void nrn_check_conc_write(Prop*, Prop*, int): Assertion `k < sizeof(long) * 8' failed. +nrnmechlibpath = "%s/demo/release" % h.neuronhome() +print(nrnmechlibpath) +assert load_mechanisms(nrnmechlibpath) + +# ca_ion now exists and has a mechanism index > nion +assert exists("ca_ion") +mt = h.MechanismType(0) # new mechanisms do not appear in old MechanismType +mt.select("ca_ion") +assert mt.selected() > nion + + +# return stderr output resulting from sec.insert(mechname) +def mech_insert(sec, mechname): + # capture stderr + old_stderr = sys.stderr + sys.stderr = mystderr = io.StringIO() + + sec.insert(mechname) + + sys.stderr = old_stderr + return mystderr.getvalue() + + +# test if can use the high mechanism index cadifpmp (with USEION ... WRITE cai) along with the high mechanims index ca_ion +s = h.Section() +s.insert("cadifpmp") +h.finitialize(-65) +# The ion_style tells whether cai or cao is being written +istyle = int(h.ion_style("ca_ion", sec=s)) +assert istyle & 0o200 # writing cai +assert (istyle & 0o400) == 0 # not writing ca0 +# unfortunately, at present, uninserting does not turn off that bit +s.uninsert("cadifpmp") +assert s.has_membrane("cadifpmp") is False +assert int(h.ion_style("ca_ion", sec=s)) & 0o200 +# nevertheless, on reinsertion, there is no warning, so can't test the +# warning without a second mechanism that WRITE cai +assert mech_insert(s, "cadifpmp") == "" +assert s.has_membrane("cadifpmp") +# uninsert again and insert another mechanism that writes. +s.uninsert("cadifpmp") +assert mech_insert(s, "capmpr") == "" # no warning +assert mech_insert(s, "cadifpmp") != "" # now there is a warning. + +# more serious test +# several sections with alternating cadifpmp and capmpr +del s +secs = [h.Section() for i in range(5)] +for i, s in enumerate(secs): + if i % 2: + assert mech_insert(s, "capmpr") == "" + else: + assert mech_insert(s, "cadifpmp") == "" + +# warnings due to multiple mechanisms at same location that WRITE cai +assert mech_insert(secs[2], "capmpr") != "" +assert mech_insert(secs[3], "cadifpmp") != "" + +############################################