ENH/REF: Feature planning: Removing celltype hardcoding

[asoplata]

@katduecker @ntolley @jasmainak @rythorpe @dylansdaniels

Let's use this Issue to coordinate the work needed for both de-hardcoding celltypes and allowing a user to provide their own.

Growing list of things that a custom celltype is required to provide:

• Network.cell_types, which is a dictionary where
  • keys are their celltype name, and
  • values are a single instance of the cell.py::Cell object describing the celltype
    • the Cell object is what it sounds like, and where you have to set sections, biophysics, etc.
• Positioning, including
  • Network.pos_dict (position dictionary, which is normally set during Network.__init__())
  • Network._inplane_distance and Network._layer_separation, either providing their own or using our hardcoded values
• What else???

Related tasks:

• Clarify the relationship between CellResponse and Network: who owns the cell type info, and how is the info communicated?
  • This is potentially a separate issue unto itself. In-person, this has been raised as a general API issue in the past.
  • Austin thinks that Network should own all celltype information (including per-celltype coloring for plots), and CellResponse should be able to access the celltype info of its "parent" Network. Reasons include:
    1. Network already requires having cell_types attributes
    2. Every instantiation of CellResponse belongs as an attribute to a specific Network (see https://github.com/jonescompneurolab/hnn-core/blob/master/hnn_core/parallel_backends.py#L46 ), except for the deprecated cell_response.py::read_spikes().
    3. This would also allow all the CellResponse-requiring plotting functions to have access to the Network object, which it use to determine canonical colors for both celltypes and drives, AND understand which of CellResponse's spike_types are celltypes as opposed to drives.
• Refactor CellResponse.cell_types (see first comment, ENH/REF: Feature planning: Removing celltype hardcoding #972 (comment) )
  • This should first be given a DeprecationWarning before it is removed.
• Set plot colors consistently, across celltypes and drives.
  • Colors should be set in a consistent manner for viz.py::plot_spikes_hist(), viz.py::plot_spikes_raster(), and viz.py::plot_cells(). In these cases, it may make the most sense to have some kind of colors dictionary attribute in Network for every unique spike type in its resulting CellResponse children.
• Harmonize between the Cell.name attributes and Network/etc. level cell type names. Also, provide a way to edit Cell.name.
• Make a "test" non-default celltype, and test its import and usage.
• What else???

Note that this Issue is limited to the API only; we have no plans to support generalized celltypes in the GUI.

Please ignore what I said about using a "feature-branch" in #970 or in-person. Let's just do things the standard way, with multiple smaller PRs into master branch.

Note that the first comment in this Issue will be frequently re-edited to keep track of what needs to be done. Feel free to edit it yourself if you think of related tasks we need to include.

[asoplata]

As I've been going through #970 and trying to get it back to passing all tests, I've noticed a minor "bug" so to speak, that CellResponse's handling of its cell_types is inconsistent, and does not only include "cell types". CellResponse.cell_types() returns a list of the unique labels of self.spike_types, but judging from both the docstring for that attribute and some test code, that list of labels includes both "cell types" which have spiked and drives which have spiked. Here is some example code that illustrates this:

#!/usr/bin/env python

from hnn_core import jones_2009_model, MPIBackend, simulate_dipole
net = jones_2009_model()
location = 'proximal'
burst_std = 20
weights_ampa_p = {'L2_pyramidal': 1.0, 'L5_pyramidal': 1.0}
syn_delays_p = {'L2_pyramidal': 0.1, 'L5_pyramidal': 1.}

net.add_bursty_drive(
    'alpha_prox', tstart=50., burst_rate=10, burst_std=burst_std, numspikes=2,
    spike_isi=10, n_drive_cells=10, location=location,
    weights_ampa=weights_ampa_p, synaptic_delays=syn_delays_p, event_seed=284)

# When I run this longer simulation, I get the following, which includes the drive:
# net.cell_response.cell_types are ['L2_basket', 'L2_pyramidal', 'L5_basket', 'L5_pyramidal', 'alpha_prox']

with MPIBackend(n_procs=8):
    dpl = simulate_dipole(net, tstop=310., n_trials=1)

print(f'net.cell_response.cell_types are {net.cell_response.cell_types}')

# When I run this shorter simulation, I get the following, which is missing 'L5_basket':
# net.cell_response.cell_types are ['L2_basket', 'L2_pyramidal', 'L5_pyramidal', 'alpha_prox']

with MPIBackend(n_procs=8):
    dpl = simulate_dipole(net, tstop=30., n_trials=1)

print(f'net.cell_response.cell_types are {net.cell_response.cell_types}')

This means that CellResponse.cell_types cannot be relied on for "cell type" information, as opposed to "unique spike types" which is a better name for it (similar to how it's used here https://github.com/jonescompneurolab/hnn-core/blob/master/hnn_core/viz.py#L608 ).

I see two ways to approach this:

1. I think is the best way: We remove all "cell type" language from CellResponse's use, and instead treat all "unique spike types that have spiked during the trial" the same.
2. We split the spike data into two parts, one for spikes of true "cell types" and one for spikes of drives. This is very clunky and bad design, including that eventually, we may want to add other things besides celltypes and drives.

Regardless, we should make discriminating between "cell type" spikes and "drive" spikes very clear and easy, including in plotting. Just like with wagdy88's changes, we could force the CellResponse user to supply true cell type names, and then we would always know if a spike type is a celltype or a drive. We could then (optionally) have a default true flag for plotting functions that only plots true celltypes by default (as opposed to drives). This would also make other things easier, like informing the user who is plotting a celltype that, if it is not found but in _cell_type_names, then that celltype can't be plotted because it produced no spikes, not that it's an incorrect name, etc. We could also remove all the celltype defaults from the plotting code that CellResponse uses, and also have a mapping at initialization time between all unique spike types and colors, so the colors are consistent. Alternatively, we could move all celltype info to Network, and have any CellResponse usage consume the info from Network.