convolve spikes and times, not rate #20
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #20 +/- ##
======================================
Coverage ? 42.75%
======================================
Files ? 39
Lines ? 4283
Branches ? 0
======================================
Hits ? 1831
Misses ? 2452
Partials ? 0
Continue to review full report at Codecov.
|
halvarsu
force-pushed
the
new_rate_map
branch
from
fc45bc2 to
d9fc6c3
Compare
|
Great work! I think we should have this new method as a default. However, first we must make sure this would not affect users if anyone relies on the old method @tristanstoeber @anecc |
|
fine for me!
…On 29. jan. 2018 18:19, Mikkel Elle Lepperød wrote:
Great work! I think we should have this new method as a default. However, first we must make sure this would not affect users if anyone relies on the old method @tristanstoeber @anecc
|
|
Dont think it’s a problem for me either :)
Sendt fra min iPhone
… 29. jan. 2018 kl. 21:53 skrev Tristan Stöber ***@***.***>:
fine for me!
On 29. jan. 2018 18:19, Mikkel Elle Lepperød wrote:
> Great work! I think we should have this new method as a default. However, first we must make sure this would not affect users if anyone relies on the old method @tristanstoeber @anecc
>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
halvarsu
force-pushed
the
new_rate_map
branch
from
93fd48e to
1ab055a
Compare
halvarsu
force-pushed
the
new_rate_map
branch
2 times, most recently
from
bcd61ba to
2073e93
Compare
|
Is it time for a merge? |
| rate[np.isnan(rate)] = 0. # for convolution | ||
| from astropy.convolution import Gaussian2DKernel, convolve_fft | ||
| from astropy.convolution import Gaussian1DKernel, convolve_fft |
There was a problem hiding this comment.
Did you find the reason why this was changed from 2D to 1D @halvarsu ?
There was a problem hiding this comment.
I think it makes sense, as it is the 1D-spatial rate map function :)
There was a problem hiding this comment.
True :) I thought it was used in the 2D case as well, but I probably just remember wrong
With inspiration from this article, I switched the order of convolution and division when creating rate maps.
The old method had problems in the cases where one bin only had one time point and one spike. This happened surprisingly often resulting in unnaturally high spike rate in that bin and neighboring bins after convolution. The new method gives an average reasonably close to the exact (number of spikes/total time) for normal smoothing factor. Here an example from a rate map where the old method overshoots:
This commit currently does not change the default behaviour of spatial_rate_map. I personally think this is a better method (easier to separate grid fields afterwards), should it be the default?