Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

First version of sim test suite #64

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

wvangeit
Copy link
Collaborator

A first version of the simulator test suite as discussed here:
#59

Creating a PR, so that @pgleeson can use this branch to add his own tests according to a similar structure.

The input files for a tests are in the relevevant 'input' directory, as in:
examples/sim_tests/biophysical/one_cell/soma_cm2/input

The expected output in expected_output

The bmtk_build has the script to build the network.

In the bmtk_test there is a test_output.py file which be used in conjunction with pytest to test aspects of the output generate by the simulator (in this case bmtk).

I marked this PR as 'dontmerge' because there are still several issues that have to be solved. The most important one:
AllenInstitute/bmtk#50
In the mean time I'm running a modified version of the bmtk with this line commented out:
https://github.com/AllenInstitute/bmtk/blob/develop/bmtk/simulator/bionet/default_setters/cell_models.py#L399

There are still things that have to be fixed / completed in soma_cm2_hh

Still have to add READMEs to the relevant subdirectories.

<channelDensity id="gnabar_hh" segmentGroup="soma" ion="na" ionChannel="hh" erev="50 mV" condDensity="0.0 S_per_cm2"/>
<channelDensity id="gkbar_hh" segmentGroup="soma" ion="k" ionChannel="hh" erev="-85 mV" condDensity="0.00 S_per_cm2"/>
<channelDensity id="gl_hh" segmentGroup="soma" ion="non_specific" ionChannel="hh" erev="-80 mV" condDensity="0.005 S_per_cm2"/>
<channelDensity id="el_hh" segmentGroup="soma" ion="non_specific" ionChannel="hh" erev="-80 mV" condDensity="-80"/>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@pgleeson, a question. Is there a way to specifiy the el_hh (reversal potential NEURON hh mechanism) without putting -80 in the condDensity field ? It seems the erev="-80 mV" of the line above gets ignored.

Copy link
Contributor

Choose a reason for hiding this comment

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

@wvangeit The reversal potential for the passive component of the hh mechanism should be covered in the gl_hh, i.e. setting erev there should be sufficient for setting this, along with the conductance of the passive component.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok, then I guess I will have to check the bmtk, because as far as I could see specifying erev didn't have an effect.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

A more general question about this. How would a simulator know it has to set el_hh when erev is specified ?

Copy link
Contributor

Choose a reason for hiding this comment

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

There is no el independent of gl_hh... I'm suggesting to remove the channelDensity id="el_hh", and the existing channelDensity id="gl_hh" is sufficient to set both the conductance and rev pot of the passive/leak conductance... This has worked fine for setting the passive properties in detailed cells already, e.g. https://github.com/AllenInstitute/sonata/blob/master/examples/shared_components/biophysical_neuron_templates/nml/Cell_472363762.cell.nml#L23,
https://github.com/OpenSourceBrain/BlueBrainProjectShowcase/blob/master/NMC/NeuroML2/Soma_AllNML2.cell.nml#L69

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

Successfully merging this pull request may close these issues.

3 participants