Fix smoothing for current amplitude detection

[arnaudon]

The base_current function was using the smoothing by default, which is hardcoded to be off in the code (bad bad btw!). I make it follow the main function (and off by default). I have traces where the scipy_signal2d makes the current = [0, 0, 0, 0], hence completely screwing up the estimation of the current amplitude.

For example an IV trace with a positive current injection was used as a IV_-20 protocol to estimate the Rin, leading to Rin of -150

[codecov-commenter]

Codecov Report

Merging #139 (edac889) into master (44bb87d) will not change coverage. The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #139   +/-   ##
=======================================
  Coverage   62.36%   62.36%           
=======================================
  Files          31       31           
  Lines        2086     2086           
=======================================
  Hits         1301     1301           
  Misses        785      785           

Impacted Files	Coverage Δ
bluepyefe/ecode/tools.py	100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[DrTaDa]

Could you attach the trace you are referring to ?

[arnaudon]

you don't need, as the smoothing should be on or of everywhere at the same time also smoothing is dead code

[wvangeit]

But anyway, provide a trace where you see the issue. So that at least we can use it to add a test.

[DrTaDa]

What do you mean by "should be on or off everywhere at the same time" ?

(It is quite computationally intensive to have it on for the full traces if it is not needed for step detection, hence the current implementation.)

[arnaudon]

here is the file: trace.csv

[arnaudon]

ah no, this is not correct either, I didn't see you overwrite ton/ton with indices, so it's not 0s, but still this trace is misclassified into IV_-20, and I can't figure out why...

[DrTaDa]

I am still not sure I understand the fix. I tried replicating your issue it but I didn't succeed.

[arnaudon]

Ok, I did a better investigation, on a trace that is more clear:

with data: trace.csv

there seems to be some boundary effect of the filter, that results in a hypamp to high, and the code then thinks this is a -10 amp_rel instead of a + something. The other trace is a similar issue it should be a smaller amp_rel than what it finds (it should be close to 0):

[arnaudon]

I think this smoothing function behaves weirdly, here are more examples:

I also added the median of the data without the smoothing in blue

[arnaudon]

I think it is boundary issues from here: https://en.wikipedia.org/wiki/Median_filter

[DrTaDa]

I see, medfilt2d uses zero padding which is clearly wrong. We should use "reflect" instead.

[DrTaDa]

We could use scipy.ndimage.median_filter instead.

[arnaudon]

I just tried with repeating the signal, and it's better. I you have a more suitable filtering function, we can try!

[arnaudon]

now I'm happy, I have almost the same Rin mean and sd than proj38! Have a nice week end, and sorry for all this noise!

[DrTaDa]

Cool !

AI ran a small performance test, scipy.ndimage.median_filter is twice as fast and does not require the repeat so we can go with that.

[arnaudon]

ok, can you update this PR? I'll rerun to check before merging

[DrTaDa]

Voila !