-
Notifications
You must be signed in to change notification settings - Fork 98
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
fix arbor tests with latest arbor version #498
Changes from 8 commits
3c887d7
c1befc6
0b06f73
68f315b
0373e39
f400cbb
86e2c14
5ad7f4d
5778cd5
f9ac053
d1758ba
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -602,7 +602,12 @@ def instantiate_iclamp_stimuli(self, decor, use_labels=False): | |||||
for i, stim in enumerate(self.stimuli): | ||||||
if not isinstance(stim, stimuli.SynapticStimulus): | ||||||
if hasattr(stim, 'envelope'): | ||||||
arb_iclamp = arbor.iclamp(stim.envelope()) | ||||||
envelope = stim.envelope() | ||||||
envelope = [ | ||||||
(t * arbor.units.ms, curr * arbor.units.nA) | ||||||
for (t, curr) in envelope | ||||||
] | ||||||
arb_iclamp = arbor.iclamp(envelope) | ||||||
else: | ||||||
raise ValueError('Stimulus must provide envelope method ' | ||||||
' or be of type NrnNetStimStimulus to be' | ||||||
|
@@ -631,7 +636,7 @@ def instantiate_recordings(self, cell_model, use_labels=False): | |||||
"""Instantiate recordings""" | ||||||
|
||||||
# Attach voltage probe sampling at 10 kHz (every 0.1 ms) | ||||||
for i, rec in enumerate(self.recordings): | ||||||
for _, rec in enumerate(self.recordings): | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Keeping the index in the latest commit |
||||||
# alternatively arbor.cable_probe_membrane_voltage | ||||||
arb_loc = rec.location.acc_label() | ||||||
if isinstance(arb_loc, list) and len(arb_loc) != 1: | ||||||
|
@@ -648,7 +653,9 @@ def instantiate_recordings(self, cell_model, use_labels=False): | |||||
|
||||||
cell_model.probe('voltage', | ||||||
arb_loc.ref if use_labels else arb_loc.loc, | ||||||
frequency=10) # could be a parameter | ||||||
"0", # tag: default is '0' | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Better:
Suggested change
and keep the index above. Sampling then needs to reference that tag. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done in latest commit. |
||||||
# frequency could be a parameter | ||||||
frequency=10 * arbor.units.kHz) | ||||||
|
||||||
return cell_model | ||||||
|
||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As said before using Arbor to write these might be an easier option. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a function that can write these kind of files? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, there's versions for full cable cells or just parts thereof. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That would basically mean to turn this template into code at the end of the ACC exporter by instantiating Arbor objects that are then serialized again. Could be an option - I guess it comes down to how much control you need over the export vs. learning the s-expression syntax. The original design of BluePyOpt was to make model exporting configurable (overridable by the user) through templates, hence I chose this option. In my view, most of the complexity in the ACC exporter comes from converting BluePyOpt's frontend representation to one suitable for Arbor. That last step of dumping the decor is less complicated, but requires some understanding of the format. I don't want to take any particular side, though. Would only suggest that if you plan to make major changes to the ACC exporter, maybe a good idea would be to do that in a separate PR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I'd support splitting this. One straight port that can be verified and a lateral move to another scheme.
This is why I as an outsider should have an opinion here :D |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
(arbor-component | ||
(meta-data (version "0.1-dev")) | ||
(meta-data (version "0.9-dev")) | ||
(decor | ||
{%- for mech, params in global_mechs.items() %} | ||
{%- if mech is not none %} | ||
|
@@ -10,7 +10,7 @@ | |
{%- endif %} | ||
{%- else %} | ||
{%- for param in params %} | ||
(default ({{ param.name }} {{ param.value }})) | ||
(default ({{ param.name }} {{ param.value }} (scalar 1.0))) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I won't check all of those in detail, but this is correct for a fixed value. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The tests are green There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd recommend to additionally check the major examples' notebooks qualitatively by inspection (not everything is run as part of the test suite). |
||
{%- endfor %} | ||
{%- endif %} | ||
{%- endfor %} | ||
|
@@ -27,13 +27,13 @@ | |
{%- endif %} | ||
{%- else %} | ||
{%- for param in params %} | ||
(paint {{loc.ref}} ({{ param.name }} {{ param.value }})) | ||
(paint {{loc.ref}} ({{ param.name }} {{ param.value }} (scalar 1.0))) | ||
{%- endfor %} | ||
{%- endif %} | ||
{%- endfor %} | ||
|
||
{%- for synapse_name, mech_params in pprocess_mechs[loc].items() %} | ||
(place {{loc.ref}} (synapse (mechanism "{{ mech_params.mech }}" {%- for param in mech_params.params %} ("{{ param.name }}" {{ param.value }}){%- endfor %})) "{{ synapse_name }}") | ||
(place {{loc.ref}} (synapse (mechanism "{{ mech_params.mech }}" {%- for param in mech_params.params %} ("{{ param.name }}" {{ param.value }} (scalar 1.0)){%- endfor %})) "{{ synapse_name }}") | ||
{%- endfor %} | ||
|
||
{%- endfor %})) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,10 @@ | ||
(arbor-component | ||
(meta-data (version "0.1-dev")) | ||
(meta-data (version "0.9-dev")) | ||
(decor | ||
(default (gSKv3_1bar_SKv3_1 65)) | ||
(paint (region "soma") (gSKv3_1bar_SKv3_1 65)) | ||
(paint (region "soma") (gSKv3_1bar_SKv3_1 65)) | ||
(default (gSKv3_1bar_SKv3_1 65 (scalar 1.0))) | ||
(paint (region "soma") (gSKv3_1bar_SKv3_1 65 (scalar 1.0))) | ||
(paint (region "soma") (gSKv3_1bar_SKv3_1 65 (scalar 1.0))) | ||
(paint (region "dend") (density (mechanism "BBP::Ih"))) | ||
(paint (region "apic") (gSKv3_1bar_SKv3_1 65)) | ||
(paint (region "apic") (gSKv3_1bar_SKv3_1 65)) | ||
(paint (region "apic") (gSKv3_1bar_SKv3_1 65 (scalar 1.0))) | ||
(paint (region "apic") (gSKv3_1bar_SKv3_1 65 (scalar 1.0))) | ||
(paint (region "apic") (density (mechanism "BBP::Ih"))))) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
(arbor-component | ||
(meta-data | ||
(version "0.1-dev")) | ||
(version "0.9-dev")) | ||
(morphology | ||
(branch 0 -1 | ||
(segment 0 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
(arbor-component | ||
(meta-data (version "0.1-dev")) | ||
(meta-data (version "0.9-dev")) | ||
(decor | ||
(paint (region "soma") (membrane-capacitance 0.01)) | ||
(paint (region "soma") (membrane-capacitance 0.01 (scalar 1.0))) | ||
(paint (region "soma") (density (mechanism "default::pas"))) | ||
(place (locset "somacenter") (synapse (mechanism "default::expsyn" ("tau" 10))) "expsyn"))) | ||
(place (locset "somacenter") (synapse (mechanism "default::expsyn" ("tau" 10 (scalar 1.0)))) "expsyn"))) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
(arbor-component | ||
(meta-data | ||
(version "0.1-dev")) | ||
(version "0.9-dev")) | ||
(morphology | ||
(branch 0 -1 | ||
(segment 0 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
(arbor-component | ||
(meta-data | ||
(version "0.1-dev")) | ||
(version "0.9-dev")) | ||
(morphology | ||
(branch 0 -1 | ||
(segment 0 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
(arbor-component | ||
(meta-data | ||
(version "0.1-dev")) | ||
(version "0.9-dev")) | ||
(morphology | ||
(branch 0 -1 | ||
(segment 0 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
(arbor-component | ||
(meta-data (version "0.1-dev")) | ||
(meta-data (version "0.9-dev")) | ||
(decor | ||
(paint (region "soma") (membrane-capacitance 0.01)) | ||
(paint (region "soma") (membrane-capacitance 0.01 (scalar 1.0))) | ||
(paint (region "soma") (density (mechanism "default::hh" ("gnabar" 0.10299326453483033) ("gkbar" 0.027124836082684685)))))) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
(arbor-component | ||
(meta-data | ||
(version "0.1-dev")) | ||
(version "0.9-dev")) | ||
(morphology | ||
(branch 0 -1 | ||
(segment 0 | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be easier by using
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in latest commit