BindsNET / bindsnet

Simulation of spiking neural networks (SNNs) using PyTorch.
GNU Affero General Public License v3.0
1.51k stars 331 forks source link

Integrating n-gram #84

Closed djsaunde closed 6 years ago

djsaunde commented 6 years ago

@hqkhan, when you have the time, can you put together a pull request with the ngram functionality in it?

hqkhan commented 6 years ago

Absolutely. I submitted a test to swarm2 yesterday to test whenever it's working or not. I should be making a pull request tonight most likely.

djsaunde commented 6 years ago

Awesome, let me know whether you need help with this.

djsaunde commented 6 years ago

Hey @hqkhan,

Any movement on this? Let me know if you need any help.

hqkhan commented 6 years ago

Hey. I'm still stuck at the bug that I was previously mentioning with the spiking activity suddenly being nothing. I tried making some counter-measures but for some reason it still ends up crashing.

Also, there's this weird thing where the ngram accuracy currently is always 100%. I tried checking predictions vs truth but they're actually the exact same. So, I'm confused.

Haven't been able to work on this for a couple of days though.

djsaunde commented 6 years ago

Okay, I'll try my best to look into it.

I think the n-gram accuracy is 100% because you're testing on the n-gram training data; i.e., you're updating the n-grams with the same data you're classifying. I could be wrong.

djsaunde commented 6 years ago

I rearranged the ngram_scores update to after the ngram_pred = ... call, which fixed the 100% accuracy all the time as expected. However, I'm running into a problem with the ngram scores calculation:

I'm looking at update_ngram_scores, and it seems like you're keep around a dictionary mapping from 2-tuples of time points to a vector of label counts.

I don't think this is what we want, right? We want to map tuples of neuron IDs to vectors of counts, I believe. I'm trying to rewrite the function to do this.

djsaunde commented 6 years ago

These two lines in ngram() cause the crash you mentioned earlier. Just remove these, and I think it won't affect the logic.

image

djsaunde commented 6 years ago

By the way, this method doesn't exactly implement n-gram; it implements a 2-gram with variable spacing between the two considered neurons indices. For example, if you pass n=3, you'll record all pairs of spikes separated by a single intermediate spike; if you pass n=4, you'll record all pairs of spikes separated by two intermediate spikes, and so on.

Does this make sense?

hqkhan commented 6 years ago

I see. I was hoping to speed through the computation by removing timesteps with no firing at all. I'll try removing it when I'm back.

Yes! I know exactly what you mean because it was one of the things I mentioned to Hananel. The main complaint I had was with the "ngram" method itself. If n=5 then we create a window of sizes n=2, 3, 4, 5 and slide each one across our "firing order" for that timestep of an example. But in the "ngram" method, we ONLY use the largest n size to evaluate. I looked at the lmm-snn repo to check what was implemented there and although we record all the tuples of different sizes up to the maximum n size, we evaluate ONLY for the largest n. I think there should be another outer loop which should go over the different window sizes same as update_ngram_scores.

djsaunde commented 6 years ago

Hm, I wasn't involved with the initial development of ngram, so I didn't know that.

No need to fix it, I'm doing it myself (and have already a pull request to the BindsNET repo).

djsaunde commented 6 years ago

We'll have to fix the ngram implementation in the future!

hqkhan commented 6 years ago

Yes, I agree. Have you tried running it? Does it work?

djsaunde commented 6 years ago

Yup, it does! I just fixed up the diehl_and_cook_2015.py script to use the 2-gram evaluation scheme (2-gram is all that works for now!).

djsaunde commented 6 years ago

By the way, I accidentally pushed to your branch. Sorry about that...

hqkhan commented 6 years ago

Awesome! Thanks for that. Just to check, it crashes at 5k training data, not instantly. I'm assuming you ran it for a while to check?

No worries about that. I'll pull. Thanks for letting me know.

djsaunde commented 6 years ago

Well, it can crash at any point. It depends on essentially all hyperparameters. I didn't run until 5k to check, because I was pretty certain what was causing the error.

hqkhan commented 6 years ago

Gotcha. True that it can crash at any point but for the default set of hyperparameters, it was crashing at 5k. No worries though, I'm sure you got it.

Hananel-Hazan commented 6 years ago

Last time I check, you dont need to run until 5k to check. Instead you can train on 500 and then run the test. It should crash on the 1050~ish image

djsaunde commented 6 years ago

I believe the issue is fixed now

Hananel-Hazan commented 6 years ago

About 2-gram evaluation, Darpan found that 2 was the best score. We need to check if its the case today

djsaunde commented 6 years ago

We need to first implement general n-gram. Darpan actually only implemented 2-gram, according to @hqkhan's comments above.

Hananel-Hazan commented 6 years ago

By the way, this method doesn't exactly implement n-gram; it implements a 2-gram with variable spacing between the two considered neurons indices. For example, if you pass n=3, you'll record all pairs of spikes separated by a single intermediate spike; if you pass n=4, you'll record all pairs of spikes separated by two intermediate spikes, and so on.

Does this make sense?

No. Is it the same implementation from the old LM-SNN?

djsaunde commented 6 years ago

Yes. What I'm trying to say is, the old n-gram implementation is incorrect.

If the above doesn't make sense, try reading it again. @hqkhan saw the same problem.