[MRG] Improve cell classes #322

jasmainak · 2021-04-29T02:59:56Z

This builds on top of #319 so the diff may not be super clear until that PR is merged and I do a rebase.

The goal is to unify the Basket and Pyramidal class representation as much as possible.

TODO

Just one cell class. Everything else through functions
default_cells.py and cells.py
add synapses to section properties (along with L, diam, Ra, cm)
Figure how to combine set_geometry and create_dends
~~self.secs should go into p_dend and be a cell property~~ (another PR, I had enough of this PR)
Tests

codecov-commenter · 2021-04-29T13:27:17Z

Codecov Report

Merging #322 (1ee4a93) into master (e037747) will decrease coverage by 0.23%.
The diff coverage is 98.26%.

@@            Coverage Diff             @@
##           master     #322      +/-   ##
==========================================
- Coverage   90.54%   90.31%   -0.24%     
==========================================
  Files          14       13       -1     
  Lines        2496     2416      -80     
==========================================
- Hits         2260     2182      -78     
+ Misses        236      234       -2

Impacted Files	Coverage Δ
hnn_core/cells_default.py	`97.82% <97.82%> (ø)`
hnn_core/cell.py	`97.02% <98.50%> (+1.56%)`	⬆️
hnn_core/__init__.py	`100.00% <100.00%> (ø)`
hnn_core/network_builder.py	`92.22% <100.00%> (-0.03%)`	⬇️

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 e037747...1ee4a93. Read the comment docs.

hnn_core/params_default.py

cjayb · 2021-05-02T10:25:49Z

hnn_core/cell_classes.py

@@ -0,0 +1,295 @@
+"""Model for Pyramidal cell class."""


all cell classes included

cjayb · 2021-05-02T10:26:30Z

hnn_core/cell_classes.py

+    return dend_props
+
+
+def _get_soma_props(p_all, cell_type):


_get_soma_props_from_params?

Edit: the "from_params" is too much, but confusing that there's this and _get_basket_soma_props below.

hnn_core/cell_classes.py

cjayb · 2021-05-02T10:34:18Z

This looks awesome! Left a few notes on what to clean up. I realise this is still WIP, but liking it a lot!

jasmainak · 2021-05-03T03:05:57Z

Code is becoming simpler and simpler. I'm not done yet but if you want a sneak peek, see here. Pardon the use of nested dictionaries in some places. I'm sure they will go away or be replaced by proper objects but it's a necessary intermediate step to conceptualize the code. I also realized that with these changes, it should be pretty straightforward to write a function that ingests a cell defined in Netpyne or another specification language without losing access to the underlying Neuron objects.

jasmainak · 2021-05-04T02:33:57Z

I could do with a round of reviews here @cjayb @ntolley @rythorpe. I am going to try and wrap up this PR now

hnn_core/cell.py

ntolley · 2021-05-04T17:04:44Z

hnn_core/cell.py

+        -----
+        By default neuron uses xy plane
+        for height and xz plane for depth. This is opposite for model as a
+        whole, but convention is followed in this function ease use of gui.


Suggested change

whole, but convention is followed in this function ease use of gui.

whole, but the convention is followed in this function

for ease use of with the gui.

Not sure I understand the connection with the gui

I guess in the past the GUI had a visualization of the cells in 3D space. The z coordinate is along the trunk of the pyramidal cells (depth) but the Neuron convention is y coordinate. I just copy pasted the old comment. Feel free to suggest a better rephrasing!

hnn_core/cell.py

ntolley · 2021-05-04T17:10:00Z

hnn_core/cell.py

+            if sec.L > 100.:
+                sec.nseg = int(sec.L / 50.)
+                # make dend.nseg odd for all sections
+                if not sec.nseg % 2:


Not your change, but I am so confused by why this is necessary

See the Neuron FAQ: https://neuron.yale.edu/neuron/faq/general-questions

ntolley · 2021-05-04T17:12:38Z

hnn_core/cell.py

+
+        # Connects sections of THIS cell together.
+        for connection in topology:
+            # XXX: risky to use self.soma as default. Unfortunately there isn't


This always confused me, is it not sufficient to add self.soma to p_secs? I guess I don't fully grasp why it's risky.

If connection[0] = 'blah' then parent_sec = self.soma which is not correct. It should have failed instead. I pushed a more readable and safer version in: f1c2d9b

hnn_core/cell.py

hnn_core/cells_default.py

jasmainak · 2021-05-04T21:27:38Z

comments addressed. Let me know if something was not properly addressed etc. The peer review is super-useful since it's not always clear to me what's not clear to others. Now I hope you'll push me to write some tests as I'm lazy as everyone else ;-) p_secs should go into a Section class but another PR ...

What else is unclear in the code?

cjayb

I certainly think you should write a lot of tests ;) I think the Cell class is probably fine as it is, but the new pyramidal and basket functions deserve some scrutiny. Here's some code I wrote recently, maybe some of it is useful:

from neuron import h
from hnn_core.network_builder import load_custom_mechanisms
from hnn_core.cells_default import pyramidal, basket

load_custom_mechanisms()

h.load_file("stdrun.hoc")
h.tstop = 40
h.dt = 0.025
h.celsius = 37

l5p = pyramidal(pos=(0, 0, 0), celltype='L5_pyramidal')

h_soma_v = l5p.rec_v.record(l5p.soma(0.5)._ref_v)
h_t = h.Vector().record(h._ref_t)

stim = h.IClamp(l5p.soma(0.5))
stim.delay = 5
stim.dur = 5.
stim.amp = 2.

h.finitialize()
h.fcurrent()
h.run()

times = np.array(t.to_python())
v_soma = np.array(h_soma_v.to_python())

plt.plot(times, v_soma)  # elicits two spikes

hnn_core/cells_default.py

hnn_core/cell.py

hnn_core/cells_default.py

jasmainak · 2021-05-05T20:39:09Z

Okay did some more simplifications. No more cell.soma and cell.dends. It's all in cell.sections. Tests added. @cjayb I used your code but wasn't sure how to detect two spikes and/or why two spikes are expected. So, it's just there as a smoke test as of now. PR is good enough to go from my end.

hnn_core/cell.py

rythorpe · 2021-05-05T23:32:14Z

hnn_core/network_builder.py

-            if cell.celltype in ('L5_pyramidal', 'L2_pyramidal'):
-                self.dipoles[cell.celltype].add(cell.dipole)
+            if cell.name in ('L5Pyr', 'L2Pyr'):
+                self.dipoles[_long_name(cell.name)].add(cell.dipole)


Is it possible for us to finally consolidate the short and long name usage?

I think we certainly can and should. We just need to get to it. I would leave it for another PR though ...

Probably a good starting point for one of the undergrads

hnn_core/params_default.py

rythorpe

Looks good @jasmainak! Our next PR will strive to make cell attributes accessible from Network!

cjayb · 2021-05-06T15:47:11Z

Thanks for the huge effort in clarifying the code around `Cells @jasmainak ! CIs are green, and all three of us have had a chance to review, so I'm going to go ahead and merge. I have a feeling new/more tests will arise in #150 and after, smoke test is fine here for now.

jasmainak · 2021-05-06T15:57:51Z

Perfect, I'll sit quite for a bit now while you guys make some PRs ;-) Thanks for reviewing!

jasmainak force-pushed the improve_cell_classes branch 5 times, most recently from 20c71da to 27fa011 Compare April 29, 2021 12:41

rythorpe reviewed Apr 29, 2021

View reviewed changes

hnn_core/params_default.py Outdated Show resolved Hide resolved

jasmainak force-pushed the improve_cell_classes branch 6 times, most recently from ae3a2c2 to a151d0f Compare May 1, 2021 01:07

cjayb reviewed May 2, 2021

View reviewed changes

hnn_core/cell_classes.py Outdated

@@ -0,0 +1,295 @@

"""Model for Pyramidal cell class."""

Copy link

Collaborator

cjayb May 2, 2021

Choose a reason for hiding this comment

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

all cell classes included

cjayb reviewed May 2, 2021

View reviewed changes

hnn_core/cell_classes.py Outdated Show resolved Hide resolved

cjayb reviewed May 2, 2021

View reviewed changes

hnn_core/cell_classes.py Outdated Show resolved Hide resolved

cjayb reviewed May 2, 2021

View reviewed changes

hnn_core/cell_classes.py Outdated Show resolved Hide resolved

cjayb mentioned this pull request May 2, 2021

explore response properties of individual HNN network cells #323

Open

4 tasks

ntolley reviewed May 4, 2021

View reviewed changes

hnn_core/cell.py Outdated Show resolved Hide resolved

ntolley reviewed May 4, 2021

View reviewed changes

hnn_core/cell.py Outdated Show resolved Hide resolved

ntolley reviewed May 4, 2021

View reviewed changes

cjayb reviewed May 4, 2021

View reviewed changes

hnn_core/cell.py Outdated Show resolved Hide resolved

hnn_core/cell.py Outdated Show resolved Hide resolved

hnn_core/cell.py Outdated Show resolved Hide resolved

hnn_core/cells_default.py Outdated Show resolved Hide resolved

hnn_core/cells_default.py Outdated Show resolved Hide resolved

jasmainak force-pushed the improve_cell_classes branch from f2222cc to f1c2d9b Compare May 4, 2021 20:29

jasmainak added 6 commits May 5, 2021 07:56

ENH: allow callable in mech value

f15693d

DOC: better documentation

d2a671d

ENH: rename cell_classes to cells_default.py

7a2fa56

MAINT: more failsafe connection of sections

9fd39bc

MAINT: no more self.secs

bb763cf

ENH: sec_pts is documented + sect_loc is updated transparently?

4958167

jasmainak force-pushed the improve_cell_classes branch from 8d2d27d to 4958167 Compare May 5, 2021 11:57

cjayb reviewed May 5, 2021

View reviewed changes

jasmainak added 2 commits May 5, 2021 08:13

FIX some quick fixes

2ff530f

MAINT: unify celltype, cell_type, and cell_name

9557f02

jasmainak force-pushed the improve_cell_classes branch from f609e1c to 9557f02 Compare May 5, 2021 19:17

jasmainak added 3 commits May 5, 2021 15:30

Clearer code?

cefc434

MAINT: get rid of self.soma

6b76eec

TST: add tests for cell class

5d38fec

jasmainak force-pushed the improve_cell_classes branch from b6b8dc0 to 5d38fec Compare May 5, 2021 20:17

jasmainak changed the title ~~WIP Improve cell classes~~ [MRG] Improve cell classes May 5, 2021

jasmainak force-pushed the improve_cell_classes branch from f730ebc to b1ac056 Compare May 5, 2021 20:41

TST: some basic tests for default cell type

abac3b2

jasmainak force-pushed the improve_cell_classes branch from b1ac056 to abac3b2 Compare May 5, 2021 21:43

rythorpe reviewed May 5, 2021

View reviewed changes

hnn_core/cell.py Outdated Show resolved Hide resolved

rythorpe reviewed May 5, 2021

View reviewed changes

hnn_core/cell.py Show resolved Hide resolved

rythorpe reviewed May 5, 2021

View reviewed changes

hnn_core/params_default.py Outdated Show resolved Hide resolved

ENH: address ryan comments + simplification

1ee4a93

rythorpe approved these changes May 5, 2021

View reviewed changes

cjayb merged commit 580fee6 into jonescompneurolab:master May 6, 2021

jasmainak mentioned this pull request May 9, 2021

separating the geometry of cells/networks from the Neuron code #165

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MRG] Improve cell classes #322

[MRG] Improve cell classes #322

jasmainak commented Apr 29, 2021 •

edited

codecov-commenter commented Apr 29, 2021 •

edited

cjayb May 2, 2021

cjayb May 2, 2021 •

edited

cjayb commented May 2, 2021

jasmainak commented May 3, 2021 •

edited

jasmainak commented May 4, 2021

ntolley May 4, 2021

jasmainak May 4, 2021

ntolley May 4, 2021

jasmainak May 4, 2021

ntolley May 4, 2021 •

edited

jasmainak May 4, 2021 •

edited

jasmainak commented May 4, 2021

cjayb left a comment

jasmainak commented May 5, 2021

rythorpe May 5, 2021

jasmainak May 5, 2021

jasmainak May 5, 2021

rythorpe left a comment

cjayb commented May 6, 2021

jasmainak commented May 6, 2021 •

edited

	whole, but convention is followed in this function ease use of gui.
	whole, but the convention is followed in this function
	for ease use of with the gui.

[MRG] Improve cell classes #322

[MRG] Improve cell classes #322

Conversation

jasmainak commented Apr 29, 2021 • edited

codecov-commenter commented Apr 29, 2021 • edited

Codecov Report

Choose a reason for hiding this comment

cjayb May 2, 2021 • edited

Choose a reason for hiding this comment

cjayb commented May 2, 2021

jasmainak commented May 3, 2021 • edited

jasmainak commented May 4, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ntolley May 4, 2021 • edited

Choose a reason for hiding this comment

jasmainak May 4, 2021 • edited

Choose a reason for hiding this comment

jasmainak commented May 4, 2021

cjayb left a comment

Choose a reason for hiding this comment

jasmainak commented May 5, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rythorpe left a comment

Choose a reason for hiding this comment

cjayb commented May 6, 2021

jasmainak commented May 6, 2021 • edited

jasmainak commented Apr 29, 2021 •

edited

codecov-commenter commented Apr 29, 2021 •

edited

cjayb May 2, 2021 •

edited

jasmainak commented May 3, 2021 •

edited

ntolley May 4, 2021 •

edited

jasmainak May 4, 2021 •

edited

jasmainak commented May 6, 2021 •

edited