Skip to content

Bug fix for detecting ION channel type in LoadBalance#2310

Merged
pramodk merged 5 commits into
masterfrom
pramodk/loadbal-iontype
Apr 20, 2023
Merged

Bug fix for detecting ION channel type in LoadBalance#2310
pramodk merged 5 commits into
masterfrom
pramodk/loadbal-iontype

Conversation

@pramodk

@pramodk pramodk commented Mar 29, 2023

Copy link
Copy Markdown
Member
  • loadbal.hoc was checking the existance of _ion to determine if a given mechanism is ion type.
  • This was causing a runtime error if SUFFIX has names like SUFFIX internal_ions like:
... internal_ions  is not an ion 0 LoadBalance[0].ion_style("internal_ions", 3, 2, 1, 1, 0)
  • with this PR, introduce an is_ion() method in MechanismType class to robustly check if a particular channel is of type ION.
  • add python & hoc test

NOTE: Ref: internal slack discussion

pramodk added 2 commits March 29, 2023 15:30
 - loadbal.hoc was checking existance of `_ion` to
   determine if a given mechanism is ion type.
 - this was causing runtime error if `SUFFIX` has
   names like `SUFFIX internal_ions` was causing a runtime
   error like:

   ```
   ... internal_ions  is not an ion
   0 LoadBalance[0].ion_style("internal_ions", 3, 2, 1, 1, 0)
   ```
 - with this PR, introduce a `is_ion()` method is MechanismType
   class to robustly check if a particular channel is of type
   ION.
Comment thread src/nrniv/nrnmenu.cpp
@azure-pipelines

Copy link
Copy Markdown

✔️ 67078f2 -> Azure artifacts URL

@bbpbuildbot

This comment has been minimized.

@bbpbuildbot

This comment has been minimized.

@nrnhines nrnhines left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Tested this with

from neuron import h
 
h.ion_register("hello_ion_foo", -3)    

def test():
    mt = h.MechanismType(0)
    sref = h.ref("")
    for i in range(mt.count()):
        mt.select(i)
        mt.selected(sref)
        print (i, sref[0], mt.is_ion())

test()

and it gives the correct output

0 morphology False
1 capacitance False
2 pas False
3 extracellular False
4 fastpas False
5 na_ion True
6 k_ion True
7 hh False
8 hello_ion_foo_ion True

Only missing thing is a doc in docs/python/modelspec/programmatic/mechanism.rst

@codecov

codecov Bot commented Apr 20, 2023

Copy link
Copy Markdown

Codecov Report

Merging #2310 (d046c0d) into master (a5a15c2) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #2310   +/-   ##
=======================================
  Coverage   58.79%   58.79%           
=======================================
  Files         625      625           
  Lines      119912   119918    +6     
=======================================
+ Hits        70498    70504    +6     
  Misses      49414    49414           
Impacted Files Coverage Δ
src/nrniv/nrnmenu.h 0.00% <ø> (ø)
src/nrniv/nrnmenu.cpp 45.43% <100.00%> (+0.44%) ⬆️

... and 2 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@azure-pipelines

Copy link
Copy Markdown

✔️ afb6505 -> Azure artifacts URL

@azure-pipelines

Copy link
Copy Markdown

✔️ d046c0d -> Azure artifacts URL

@pramodk pramodk merged commit 1d8cf50 into master Apr 20, 2023
@pramodk pramodk deleted the pramodk/loadbal-iontype branch April 20, 2023 08:59
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.

4 participants