Skip to content

Conversation

@djsaunde
Copy link
Collaborator

@hqkhan, notice that I've made some changes to your plot_spikes() function. Namely, I

  • Changed indentation from spacing to tabs
  • Favored object oriented matplotlib calls (e.g., plt.subplots())
  • Changed doc string layout

Otherwise, it's a great implementation. I mostly wanted the code to fit into the overall style of the library.

@hqkhan
Copy link

hqkhan commented Feb 28, 2018

Not a problem at all. I'll definitely be trying to stay more consistent with the code style. I just looked into the changes you made. It looks good. I'll be trying my best to adopt the style for when I implement general plotting function using monitors and keywords a we discussed before (after my exam today). I just have one concern. Your axis don't have any values to them. Is it not important to be able to see which neurons are spiking at which time? Technically, we can still do that but we would have to count it ourselves.

@djsaunde
Copy link
Collaborator Author

djsaunde commented Feb 28, 2018

It would be nice to see which neurons are spiking at which times, but depending on what's being plotted, the axis ticks could become quite cluttered. For example, imagine a simulation for 10,000ms (10 seconds) and the corresponding tick marks. In the old code, there would be 10,000 / 5 = 2,000 tick marks on a figure only 12 inches wide, giving one tick mark every 0.006 inches.

Even with tick marks, it is still not very clear when certain spikes have happened, depending on the frequency of the tick marks.

A good compromise would be to make the tick mark frequency a parameter. For now, I prefer no tick marks at all, since it's neater and the plots are still instructive if the user knows how long the recording is.

@hqkhan
Copy link

hqkhan commented Feb 28, 2018

Yup, I completely agree and we can leave it at no tick marks for now. However, I'm interested in trying frequency being a parameter. The hard-coded 5 from before was sort of just a test. I was going to look into making it dynamic and adjust according to the time as you're mentioning. I think plotting 10sec worth of spikes might produce a cluttered graph altogether which is where the time parameter would help. Instead, one could divide up the times by let's say 2000ms per figure and plot those. The frequency parameter could then take up a nifty little formula (I don't know what yet).

@hqkhan
Copy link

hqkhan commented Feb 28, 2018

So, now we have working functions that plot different things of the network. Are we still trying to look having a general plotting function? With keywords, the general plotting function could call the current functions instead?

@djsaunde
Copy link
Collaborator Author

Yeah, I think it would be great to have. I think the specific functions are great for our particular experiments (to reduce cognitive strain), but a general one could do plt.imshow() or plt.matshow() on some recording of real valued data from a Monitor object. Keyword arguments might include cmap (one of [binary, hot, hot_r, etc...]), your time=(start, end) argument, figsize, xlabel, ylabel, title, ..., which would all default to sensible values (e.g., xlabel / ylabel / title could be empty).

@hqkhan
Copy link

hqkhan commented Feb 28, 2018

Cool. Another thing that I was thinking of is to include the number of neurons parameter for plotting (not sure if necessary). If we use 6400 neurons, that'd be a pretty big plot. So, it'd work same as time and plot the desired set of neurons to have cleaner plots to look at.

@djsaunde
Copy link
Collaborator Author

Great idea!

@djsaunde djsaunde merged commit 1277e62 into master Mar 1, 2018
Hananel-Hazan pushed a commit that referenced this pull request Jun 19, 2019
fixed indexing of davis dataset
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants