Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AP_end_indices needs some attention #88

Open
wvangeit opened this issue Jan 17, 2017 · 13 comments
Open

AP_end_indices needs some attention #88

wvangeit opened this issue Jan 17, 2017 · 13 comments

Comments

@wvangeit
Copy link
Contributor

As a followup on an issue encountered by @a1eko, we should look deeper into the AP_end_indices feature. AP_begin_indices was reimplemented in LibV5, but we also need a clean up version of AP_end_indices.

@wvangeit
Copy link
Contributor Author

We should also add detailed documentation about how these features work

@elisabettai
Copy link

Hi,
I just want to bring up this problem with the computation of AP_end_indices. In this trace for example, it seems to me that the values of AP_end_indices are really off. This is a bit problematic I think, because features such as "AP_duration" and "AP_duration_half_width" depend on that (as I read here).

Screenshot 2020-11-26 at 09 09 33

trace.txt

import matplotlib.pyplot as plt
import efel
import numpy as np
trace = np.loadtxt("trace.txt")
etrace = {"T":trace[:,0], "V":trace[:,1], "stim_start":[800], "stim_end":[1000]}

efel.setDoubleSetting('Threshold', -30)
feats = efel.getFeatureValues([etrace], ['AP_end_indices','AP_begin_indices',
                                    'time', 'voltage'])[0]

plt.plot(etrace["T"],etrace["V"])
plt.scatter(feats['time'][feats['AP_begin_indices']],feats['voltage'][feats['AP_begin_indices']],
            label='AP_begin_indices',color="g" )
plt.scatter(feats['time'][feats['AP_end_indices']],feats['voltage'][feats['AP_end_indices']],
            label='AP_end_indices',color="r" )
plt.legend()

@DrTaDa

@wvangeit
Copy link
Contributor Author

It looks like this behavior is caused by the relatively slow drop of the voltage after the peak. eFEL is using a derivative threshold to detect the spike end. That value is hardcoded atm (should be changed) at -12, when I change it to e.g. -5, I get:

ap_end

So unless we rethink the entire feature, the solution seems to be to let the user change that threshold. Is that ok for you?

@a1eko
Copy link

a1eko commented Nov 26, 2020 via email

@elisabettai
Copy link

elisabettai commented Nov 26, 2020

A user-defined derivative threshold could work, it's just a bit hard to know a priori what a good value might be.

Another way that I found working for this trace is finding 'AP_end_indices' as the first index where voltage crosses the threshold after the peak. This is I think what experimentalists normally do to compute AP duration and AP duration at half height of the action potential. In this way we only need 'AP_begin_indices' (and derivative threshold) and 'peak_indices', which are calculated correctly for this case. However, as you said @wvangeit, I suspect that this would change the behaviour completely.

@wvangeit
Copy link
Contributor Author

Ok, so let's start with the user-definied derivate threshold then.
@anilbey , can you maybe have a look? I think it's best to make a new version of AP_end_indices in LibV5 and make it similar to AP_begin_indices. It should also accept a threshold for the derivative. I think it's best we accept a separate value for the downward derivative.

@anilbey
Copy link
Contributor

anilbey commented Jan 19, 2021

What can be a good default value for the downward derivative?

wvangeit added a commit that referenced this issue Jan 21, 2021
add AP end indices with a variable derivative threshold in LibV5 #88
@anilbey
Copy link
Contributor

anilbey commented Jan 21, 2021

In #196, the AP_end_indices work exactly like in V2 and it accepts a variable threshold.

With @AurelienJaquier we also did a re-implementation similar to the V5::AP_begin_indices but that changes the results of V2 features depending on the AP_end_indices like fall_rate. Should we apply that update?

@wvangeit
Copy link
Contributor Author

What change in implementation makes these value differ? Is it a significant difference?

@AurelienJaquier
Copy link
Collaborator

The V2::AP_end_indices just takes the 1st value to be above threshold. When we make it similar to V5::AP_begin_indices, it takes the 1st value only when all next values in a given window (e.g. all 4 next values) are also above threshold.
Most values ended up the same, but here are some that ended up different:

AP_end_indices: 9114->9138
AP_fall_indices: 9113-> 9127
AP_duration_change: -0.515 -> 0.212
AP_duration: 1.9 -> 4.0
AP_fall_rate: -4.75 -> -17.34
AP_fall_time: AP_fall_time: 0.1 -> 2.5
AP_duration_half_width: 0.9 -> 2.3
feature depolarized_base: -40.9 -> -41.1

So I guess it can create some significant differences on some features.

@wvangeit
Copy link
Contributor Author

Ok, not sure we should add this new implementation with the same name in eFEL atm. It will change all the results of people running optimizations etc. Maybe we can keep it in mind for the new major eFEL version?

@a1eko
Copy link

a1eko commented Jan 28, 2021

I think, it would make sense to keep the old default values for backward compatibility but make the threshold user-defined. If a significantly new algorithm is going to be implemented, then perhaps the feature deserves a new name (like adaptation_index and adaptation_index2).

@wvangeit
Copy link
Contributor Author

Ok, sure, #196 has made the threshold user-definable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants