hnn-core
hnn-core copied to clipboard
WIP: Upgrade plot_morphology
Closes #496
Goals:
- Create new
viz.py
and correspondingnet.py
method to plot morphologies of the cells within a network:plot_cell_morphologies
- Move common functionality of plotting cell morphologies in
plot_cell_morphology
and (new)plot_cell_morphologies
to private method inviz.py
:_plot_cell
So far, the three new methods are setup to discuss code architecture, but not complete.
Looks good so far! Let's see how it shapes when you have it working
Looks good so far! Let's see how it shapes when you have it working
Agreed! Also @mjpelah I added the tag "WIP" before the title to indicate the PR is still a work in progress.
One idea I pitched to @mjpelah was to allow viz.plot_cell_morphology
to accept a Cell
or list of Cell
as it's first argument so that Cell.plot_morphology
and Network.plot_cell_morphology
are both wrappers for the same viz.py
function. This would maximize the flexibility of viz.plot_cell_morphology
so that users can plot an arbitrary number of cells on the same axis if they wish (myself being a user who might wish to do this). Any thoughts on or objections to this?
I like that! Would you have another argument that specifies the soma locations?
Current iteration has three functions in viz.py
:
-
plot_cell_morphology
-
plot_cell_morphologies
-
_plot_cell
Output for plot_cell_morphologies
is:
As you can see, the cells are not offset (next step) as only the L2_pyramidal and L2_basket show, with the others (presumably) behind.
I thought I would try this implementation however, after thinking it through with Ryan, I believe it would be best to have one just one function that takes either a single cell or a list (or dictionary), creates a list (if only a single cell was passed in) and iterates through the list, plotting each cell.
Thoughts?
I think it's worth thinking through how the user would use the function. If you have a single function for one cell and multiple cells, how would the offsets be plotted? Would you still plot the cell with the same z offset if it was the only cell being plotted?
I think it's worth thinking through how the user would use the function. If you have a single function for one cell and multiple cells, how would the offsets be plotted? Would you still plot the cell with the same z offset if it was the only cell being plotted?
I think the best way to handle this would be to get the morphological radius for each cell and then plot each successive cell at a distance equal to the sum of the radii for each pair of adjacent cells. Then, the user could input a Cell
instance or list
of Cell
instances without having to worry about overlapping on the same axis.
We also need the z-axis to be visible !
I think it's worth thinking through how the user would use the function. If you have a single function for one cell and multiple cells, how would the offsets be plotted? Would you still plot the cell with the same z offset if it was the only cell being plotted?
I think the best way to handle this would be to get the morphological radius for each cell and then plot each successive cell at a distance equal to the sum of the radii for each pair of adjacent cells. Then, the user could input a
Cell
instance orlist
ofCell
instances without having to worry about overlapping on the same axis.
The solution @rythorpe proposed might allow for more readable code. If you used separate functions for a cell vs cells, you would then need a private function (as in the most recent commit) because they're would be a lot shared (redundant) code between the two. This would be fine, calling the private function once for a cell and more than once (for loop) for cells, but how would you set the offset for multiple cells?
You could:
- Individually pass in the cell position when dealing with multiple cells, but might increase amount of redundant code
- Pass in the number of cells you are plotting as an int, therefore it could create the varying positions for the offset itself: this could either be done by recursively calling _plot_cell within the function or with a loop.
- Perhaps set, only, the cell position offset in
plot_cell_morphologies
, and have it carry over to_plot_cell
, however I do not know how this would be done or if it is possible
I feel like with any of these implementations, it would be difficult to read or there would be redundant code. Alternatively, to have one function that treats any cell (or cells) that are passed in, as a list of cells, and iterate through the list setting a new offset for each subsequent iteration, might be less redundant.
We also need the z-axis to be visible !
@jasmainak z-axis as well as x and y?
I thought we were first going to get something like this, without z axis:
actually is it possible to share axis between a 3D plot and a 2D plot? can you try plotting this alongside the LFP to see how it looks?
but that plot shows the z axis ...
I was thinking of two separate functions that call a common private function. In each function you pass the position ... when it's only one cell, the position is z=0, when it is called from the method of network to plot all the cell types, use the positions of the cells in the network.
but that plot shows the z axis ...
Oh yes, my mistake. I'm used to x being left right, y being up down, and z being forward back.
Try to first plot it alongside LFP with 2 subplots and see if it makes sense. Then we can make the code organization nice :)
Small nitpick, the _plot_cell()
function is a bit confusingly named as there is a plot_cells()
function which is called from the Network
and plots the soma positions.
Perhaps the next PR could merge this functionality and rename plot_cells()
to plot_somas()
or something. Then we could have an option to plot the full network with real cell morphologies as well (although it is extremely slow, trust me I checked :smile:)
I like plot_somas
!
any updates here?
any updates here?
Nothing significant. I've got a poster presentation to create for Monday on the LFP/CSD project so that is taking up most of my time
This can be saved for another PR, but two things that would be really nice to have in plot_cell_morhology
is the ability to specify different colors for each section, as well as specify the position of the cell (current the cell is moved to the (0,0,0) coordinate automatically).
To see an example of how this could be implemented you can check out the changes made here.
Just an update on progress:
- All cell morphologies are now handled by one function
- If a single cell or dict or cells is input, they are placed in a list
- The list is iterated, plotting each
For multiple cells, an offset based on the radius of the cell is created, however is not completely working yet:
Something went wrong with your git ...
And you need to make this a 2D plot or have an option for 2D plot, otherwise I don't know if you can combine it with the LFP plot.
Yeah, something is off. It almost looks like you rebased master
onto your upgrade_plot_morphology
branch instead of vice versa.
@jasmainak I had advised Matt to get the logic down first, as there are a few options we discussed yester of how to place the figure into a subplot so that it looks pretty and is visually aligned with the y-axis of an LFP plot.
okay sounds good. I just want to make sure you are thinking of that goal. Because the logic and code structure might be slightly different if you have only 2 dimensions to plot the morphology
@mjpelah looks like you need to rebase :) :) :)
@mjpelah looks like you need to rebase :) :) :)
@jasmainak Yep, just got home, have been busy with all the symposiums and presentations; will rebase and continue tomorrow!
sounds good, thank @mjpelah
@jasmainak could we meet very briefly (5-10m) to discuss some things with this PR and some issues I had with git? It would be a big help!
Sorry I just saw this email. Can you shoot me an email with a zoom id? I can join anytime now
How's it going @mjpelah? Anything we can assist with here?