Shuffle Circular-Bins is broken about spiketools

[maestapereira]

@TomDonoghue I would say yes! Feel free to close the issue.

[maestapereira]

I couldn't find any bugs in the code.
I believe the unexpected behavior we were observing is due to the argument bin_width_range.
From changing the ranges around, it looks like the values (especially the maximum) in bin_width_range should be larger. For example, in the tutorial update #115, using bin_width_range=[5000, 7000] yielded reasonable shuffles for me.

Changes to the function I would suggest:

• Updating the default of bin_width_range.
• Maybe updating the bin_width_range argument to seconds (right now it is in milliseconds).
• On line 128: shuffle_num = np.random.uniform(...).astype(int) -> np.random.randint

Lastly, on line 128: the division by 2 in ...low=bin_width_range[0] / 2 has its function but it is a little arbitrary and not necessary. I don't know how to handle this/what to suggest/if it should be changed.

Let me know which changes make sense and if I should go ahead and create a PR.

[TomDonoghue]

Huh, interesting.

Suggested updates:

• I'm definitely +1 on updating to use seconds for this
• +1 on updating the shuffle_num line (just double check the behaviour doesn't weirdly change)
• For bin_width_range, we should change the default to a more sensible one. Do you have a recommended value?
• I think we need to add a bit more description to bin_width_range (or maybe just extra notes) better describing how to choose bin_width_range. Does the value you should choose depend on the overall length of the data?

For the division by 2, when you say "has its function but it is a little arbitrary", what does this mean? What is the function? We could either keep it, and describe it, or toss it - I don't have any strong opinions for this right now.

As a tangent, is it possible / likely that when we switched the data from ms -> seconds, things got worse (because the bin argument was bad), but before that it was working okay?

[maestapereira]

I tested different values for different spike frequencies and data lengths and I would suggest something like [500, 7000] or [.5, 7] after we change the units.
I believe an adequate bin_width_range depends on the combination of data length/firing rate. It seems like neurons with higher frequency spiking are more forgiving when bin_width_range[1] - bin_width_range[0] is small (~less than 1000). Also, if the data length is very small, and a neuron is sparse, bin_width_range[0] can't be too large. More importantly, smaller bin_width_range[1] create those weird effects we were observing.

The division by 2 has its function but is a little arbitrary because it adds a larger range to the circular shuffle element. That is possible because the amount by which each randomly sized bin can be rolled can be at most bin_width_range[1], but it can be less than bin_width_range[0]. So, in practice, the line 128, currently

shuffle_num = np.random.uniform(low=bin_width_range[0] / 2, ...

could be substituted by

shuffle_num = np.random.uniform(low=bin_width_range[0] / n, ...

where n is any number, or even

shuffle_num = np.random.uniform(low=0, ...

I think that when we switched the data from ms -> seconds things got worse. Also, depending on the neuron firing rate/data length combination, the current default argument can work fine.

[TomDonoghue]

@maestapereira - is this fully addressed by #120, do you think? Or is there anything else you think we should check / do here?