[Fix] eda_peak(): Change peak detection from differentiation signal to phasic signal

[Nattapong-OnePlanet]

Dear NeuroKit developers,

We would like to notify the bug in the implementation of Kim's peak detection method.

We started with the phasic signal as generated in the test_eda_peaks() in NeuroKit/tests/tests_eda.py. With the current version of Kim's method, the slopes are usually detected at the steep slope and the peak heights are usually smaller than what they should be, as shown in the following figure.

Once this bug is fixed, the Kim method can detect correct peak heights and peak locations, as in the following figure.

We found that the value, from which the algorithm should measure peak height, should be "phasic" signal, instead of "differentiation" signal. This error leads the algorithm to find the point with maximum "slope" rather than maximum "phasic" value.

We would like to refer to Section 2.3.2 and Figure 5 in the original paper. Differentiation time-series should be used to find two consecutive zero-crossing from negative to positive and positive to negative. Then, the SCR should be the "EDA phasic" signal between these two zero-crossing points, rather than the "differentiation" signal, as currently implemented in NeuroKit. As a result, the peaks are detected at the phasic signal with maximum slope and the slope is now referred to as the yielded 'amplitude', which is much smaller than the true amplitude and thus incorrect.

We also propose to include all SCRs detected between zero-crossing differentiation points, find maximal peak amplitude and use 10% of it as the threshold, and later exclude small peaks.

Description

This PR aims at fixing the bug from measuring SCR amplitude using differentiation signal to EDA phasic signal. Furthermore, the flow of small peak exclusion is changed. Originally, the slope threshold is set as 0.1 x maximal slope in the phasic signal. This threshold is set inactive by setting to 0, in order to include all peaks. After peak detections, threshold is calculated by 0.1 x maximal peak amplitudes. The threshold is then used to mask out SCRs with small amplitude, as in the original paper.

Proposed Changes

We changed [df[zeros[i] : zeros[i + 1]]] to [eda_phasic[zeros[i] : zeros[i + 1]]]. and np.argmax(df[zeros[i] : zeros[i + 1]])] should be changed to np.argmax(eda_phasic[zeros[i] : zeros[i + 1]])].

We found out that BioSPPy (0.6.1) also uses differentiation signal as the input and similarly gives peaks at the slopes. Therefore, the peaks detected from the fixed NeuroKit can be displaced from BioSPPy for more than 1 index. In tests/tests_eda.py, we changed the atol parameter in test_eda_peaks() from 1 to 180, and passed the test with the short 30-second signals with 6 peaks. This demonstrates that the peaks are generally detected but at the shifted points, for a maximum of 0.180 seconds. For a better proof, we proposed to expand the length of the testing phasic signals and the number of peaks to 20 times, by changing duration and scr_number parameters.

Finally, the smaller peaks are excluded by comparing the amplitudes with a threshold, which is defined as 10% of all SCR amplitudes.

Nattapong @ OnePlanet

Checklist

• I have read the CONTRIBUTING file.
• My PR is targetted at the dev branch (and not towards the master branch).
• I ran the CODE CHECKS on the files I added or modified and fixed the errors.
• I have added the newly added features to News.rst (if applicable)

[welcome]

Thanks for opening this pull request! We'll make sure it's perfect before merging 🤗
Make sure to read the contributing guide. Also, if you think that your contribution is worthy of it, you can consider adding yourself to the Contributors list (feel free to ask us if you have any doubts).

[codecov-commenter]

Codecov Report

Merging #697 (74b5340) into dev (ce2516d) will increase coverage by 0.01%.
The diff coverage is 81.81%.

@@            Coverage Diff             @@
##              dev     #697      +/-   ##
==========================================
+ Coverage   52.74%   52.76%   +0.01%     
==========================================
  Files         277      277              
  Lines       12635    12640       +5     
==========================================
+ Hits         6664     6669       +5     
  Misses       5971     5971              

Impacted Files	Coverage Δ
neurokit2/eda/eda_findpeaks.py	57.52% <81.81%> (+1.96%)	⬆️

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

[DominiqueMakowski]

Hi @Nattapong-OnePlanet awesome work, thanks a lot!

[welcome]

Congrats on merging your first pull request! 🎉🍾 We're looking forward to your next one!