Open sheiksadique opened 11 years ago
The translation between physical and logical is indeed computationally inefficient, and this was the easiest solution at the time I chose to do it.
Spikelists require the neuron ids to be of hashable type. This means tuple of basic data types (not list), or any basic data type (int, float etc). So you can use physical addresses as ids, but any visualization would indeed become garbage. I would suggest to create a class that inherits from SpikeList, and that knows how to translate physical to logical. When a function requires the logical addresses they are created 'on-the-fly' (e.g. a function named logical_id_list()).
But the real problem is that you must find a way to correctly represent and visualize the multiple channels, which is why I've never done it this way.
2) btw the input object is not cryptic: it is simply a channel-spikelist dict. There is still an open issue proposing to replace this with a sequencer class, something in line with the monitor object. The sequencer object could e.g. be a container for the spikelist.
And ideally, all these changes should be made without breaking old code... :)
I would suggest to create a class that inherits from SpikeList, and that knows how to translate physical to logical. When a function requires the logical addresses they are created 'on-the-fly' (e.g. a function named logical_id_list()).
I think that's a reasonable idea. But, IF we are going to use Monitors to visualize raster plots, the monitors would ideally have all the info about address translation etc and therefore we shouldn't have to fiddle around with the SpikeList object. Wouldn't u agree ? And since Monitors are specifically designed and developed for this purpose, I think that is where this mechanism for visualization should be left.
But the real problem is that you must find a way to correctly represent and visualize the multiple channels, which is why I've never done it this way.
Since we are using Monitors now, why should we care about channels ?
2) btw the input object is not cryptic: it is simply a channel-spikelist dict.
It might not be cryptic for us since we have been working with it since the begining, BUT as a user who hasn't written down the setup files himself, one has very little info about what channels mean or why he should be fiddling around with those. And this i know because there were some users I had to explain this to.
Following some discussion with Fabio, here is what we thought was reasonable.
The stimulus that the run/stimulate function accepts is currently expected to be of the type {channel:SpikeList}. How about allowing the following.
@eneftci : What do you think ?
That is a good idea.
But do we need to have spikelists at all? For monitoring, pyST builds SpikeLists at the very end because it is not very efficient (it is a dictionary of SpikeTrains). Instead it uses the pyST.STas.events and pyST.STas.channelEvents classes. I've designed these classes to be as fast as python can be. The downside is that some basic functions (e.g. mean_rate, firing_rate) etc. will not be available, but you could easily extend pyST.STas.events to offer these functions as well.
The group.spikelist_* functions should then return events as well.
E.
In fact I'm using events for a specific use of the real time stimulator and I think it shouldn't be hard to handle those. However I favour the idea of extending events with basic functions or making it somewhat usable at the high level. A "high level" class that inherits from events could also be a possibility, e.g. Spikes.
In any case, we should make sure that every piece of the chain has consistent sanity checks for the two possible data formats, dictionary of SpikeList or events/whatever, in both the stim (generate stim and send) and mon (listen and return) pathways.
2013/2/26 eneftci notifications@github.com
That is a good idea.
But do we need to have spikelists at all? For monitoring, pyST builds SpikeLists at the very end because it is not very efficient (it is a dictionary of SpikeTrains). Instead it uses the pyST.STas.events and pyST.STas.channelEvents classes. I've designed these classes to be as fast as python can be. The downside is that some basic functions (e.g. mean_rate, firing_rate) etc. will not be available, but you could easily extend pyST.STas.events to offer these functions as well.
The group.spikelist_* functions should then return events as well.
E.
— Reply to this email directly or view it on GitHubhttps://github.com/inincs/pyNCS/issues/16#issuecomment-14096816 .
Ah getting rid of SpikeLists all together..! why not ! We just have to make sure the API remains the same ie. the function names and objects and its attributes are still closely consistent with SpikeList's functions.
So it this the plan then ? 1) Events will be used with paddr as stimulus for both input and output 2) The use of Events object will be optional in the corresponding functions, leaving {channel:SpikeLists} as default for now. 3) The Events class needs to be updated to accommodate spike analysis functions like firing rate, isi, etc.
Do we agree on the action plan ? Please add anything else that I left out here.
Well I would get completely rid of SpikeLists, but rather just use it for analysis, at least for the moment. Spikelists do have an advantage when it comes to selecting one or subsets of neurons, because dict is a hash table: pulling our a dictionary element is O(1), whereas events (which is an array) needs a preliminary sort O(nlog(n)) and additional memory for the same operation.
Consistency should not be a problem because Spikelists are always converted to events / channelEvents at some point. The channelAddressing class takes care of this. A few checks along the way should take care of this.
By extending events, I meant what Fabio said: create a new class "spikes" that inherits from events.
Also remember that channelEvents is probably the class you would like to use/extend/inherit from: it is a dictionary of events where the key indexes the channel and its corresponding value is an event object.
E.
The NeuroSetup.stimulate() function expects a dictionary of SpikeLists. The SpikeLists use logical addresses as ids. On function call, these addresses are translated to physical addresses and sent to the setup. The same happens backwards on the readout of activity from the setup.
All of this process involves considerable computation and for stimulus spanning across large populations slows down the stimulation and readout process. This is especially visible when you want to do real-time demos.
Since the LogicalAddressing is really only for the purposes of plotting, why not just use the PhysicalAddresses for creating and sending the stimulus ? This has several advantages 1) No processing required at the time of stimulation or readout. You only require to process the spikes when you are interested in visualizing the spikes. 2) The Input object used to stimulate is very cryptic as it is now {channel:SpikeList}, since the true essence of pyNCS is to not bother users about the channel information, a simple SpikeList would be better input representation. 3) This was earlier not possible since ligical addresses are not unique across a multichip setup. But with the use of PhysicalAddresses a single SpikeList object that includes all the input spikes is easily possible.
I must also emphasize that if you all agree to this suggestion, we need to make a SMOOTH transition and make it optional for now because there is a good chance that this enhancement will break old code (although it was intended for the greater good of mankind ;) ).