Skip to content

Conversation

@dsanghavi
Copy link
Contributor

Update contains:

  1. ngram in evaluation/init.py
  2. SpokenMNIST in datasets/init.py and eth_spokenMNIST.py

@dsanghavi dsanghavi requested a review from djsaunde March 7, 2018 04:45
import pickle as p
import numpy as np
import scipy.io.wavfile
from scipy.fftpack import dct
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move this down to the from imports and correct the spacing.

Data is divided by an 80-20 split into train and test
'''
def __init__(self, path=None):
self.data_dir = '/home/darpan/sem4/free-spoken-digit-dataset/recordings/'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a hard-coded path, and will fail on general machines. Take a look at the MNIST class for a default way of doing this.

class SpokenMNIST:
'''
Data is divided by an 80-20 split into train and test
'''
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More descriptive doc-string + punctuaction.

labels.append(label)
return audios, torch.Tensor(labels)

def pre_process(self, file):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe preprocess instead of pre_process?

filter_banks = 20 * np.log10(filter_banks) # dB

return filter_banks, label

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, this method seems really messy / hard-coded. Perhaps some of the parameters should be arguments to the function (e.g., NFFT, frame_size, frame_stride, etc.).

spike train from that timestep onwards
Inputs must be non-negative. Spike inter-arrival times are inversely proportional to
input magnitude, so data must be scaled according to desired spike frequency.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But, it's not a "mixture" per se, is it? I feel like the function header is misleading.


# Yield Bernoulli-distributed spike trains.
return s

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can simply add functionality to get_bernoulli in which the time dimension is implicit; e.g., we could pass time=None, and based on this, discard the explicit time dimension.

Most important functions to use:
confidence_weighting()
ngram()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dunno what this string is for. Could you remove it?

'''

def get_fire_order(example):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unclear what this function is doing without a doc string. I suggest get_firing_order.

fire_order.append(n_id)
return fire_order

def normalize_probability(v):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it necessary to put this in a separate function? In general, you should avoid proliferation of functions inside of library modules, since they can be imported by end-users.

Top-k
Precision
Recall
'''
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-write doc string to conform to style elsewhere in package.

if not tuple(seq) in ngrams:
ngrams[tuple(seq)] = np.zeros(10)
ngrams[tuple(seq)][true_label] += 1

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider folding both of these functions into the ngram function.

from nodes import AdaptiveLIFNodes, LIFNodes, Input
from analysis.plotting import plot_input, plot_spikes, plot_weights

from evaluation import *
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix spacing.

images, labels = MNIST(path='../data').get_train()
images /= (255 * min_isi) # Normalize and enforce minimum expected inter-spike interval.
images = images.view(images.size(0), -1) # Flatten images to one dimension.
labels = [int(lbl) for lbl in labels]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do this? Having the labels lazily returned from a generator is a good thing. Same with images, which should be returned as a generator as in the get_poisson and get_bernoulli functions.

print('Begin training.\n')
start = default_timer()

train_spikes = []
Copy link
Collaborator

@djsaunde djsaunde Mar 7, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider just appending spike_record to disk every update_interval. This way, we won't have to store n_samples * time * n_neurons spikes.

Copy link
Collaborator

@djsaunde djsaunde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See inline code comments.

coopersigrist added a commit that referenced this pull request Oct 29, 2019
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.

4 participants