neuropsychology / NeuroKit

NeuroKit2: The Python Toolbox for Neurophysiological Signal Processing
https://neuropsychology.github.io/NeuroKit
MIT License
1.58k stars 420 forks source link

Function ecg_delineate gives IndexError #364

Closed sitadrost closed 2 years ago

sitadrost commented 4 years ago

With some signals, ecg_delineate returns an IndexError (sometimes even dependent on the method used; e.g. 'peaks' works fine, but 'cwt' would throw an error):

Traceback (most recent call last):

  File "<ipython-input-39-b0a5d79fd400>", line 1, in <module>
    test = nk.ecg_delineate(ecgclean, sampling_rate = fs, method = 'cwt', show = True)

  File "/home/sita/Documents/Python/NeuroKit/neurokit2/ecg/ecg_delineate.py", line 133, in ecg_delineate
    instant_peaks = signal_formatpeaks(waves_noNA, desired_length=len(ecg_cleaned))

  File "/home/sita/Documents/Python/NeuroKit/neurokit2/signal/signal_formatpeaks.py", line 14, in signal_formatpeaks
    signals[feature] = _signal_from_indices(info[feature], desired_length, 1)

  File "/home/sita/Documents/Python/NeuroKit/neurokit2/signal/signal_formatpeaks.py", line 35, in _signal_from_indices
    if isinstance(indices[0], np.float):

IndexError: list index out of range

As far as I could find out up to now, it looks like the number of detected R-peaks doesn't match the value expected by the code somehow (when I do the delineation step-by-step, i.e. ecg_peaks first, and then ecg_findpeaks, a different method for ecg_peaks sometimes helps).

To Reproduce I'm afraid I can't share the signals that give me this error, because of privacy reasons. Just wondering whether you noticed this error before, and whether there's something I can do about it (apart from looking through the code and trying to figure it out myself).

System Specifications

Tam-Pham commented 4 years ago

Hi @sitadrost

Thank you for raising the issue. The cwt method is in-fact not as stable as peak and dwt. As of now, I would recommend using the 2 stable methods if possible.

As far as I could find out up to now, it looks like the number of detected R-peaks doesn't match the value expected by the code somehow (when I do the delineation step-by-step, i.e. ecg_peaks first, and then ecg_findpeaks, a different method for ecg_peaks sometimes helps).

Could you perhaps elaborate this issue a little bit more. I understand that you can't share the data but perhaps you can show us the script that you use for this investigation as well as the discrepancies that you found. This will help us to address the issue more quickly.

Thanks much!

sitadrost commented 4 years ago

Hi @Tam-Pham

Thanks for getting back, you're fast! That's good to know, about the cwt method. However, this was just an example where one method would work, and the other would give an error. So far, I've been using all default methods mostly.

I'm afraid there's not much to the script that I'm using: it's just calling ecg_delineate that's sometimes throwing errors, so for example

import neurokit2 as nk
ecg = <load some signal>
fs = 100 
signals, info = nk.ecg_process(ecg, sampling_frequency = fs)

where ecg_delineateis called as a part of ecg_process, and gives the error that I showed in my initial post.

sitadrost commented 4 years ago

Ah sorry, I get what you were asking now! However, by now I've been messing around so much that I can't seem to reproduce that particular behaviour (different signal? or perhaps I just remembered wrong?). Terribly sorry about that!

Tam-Pham commented 4 years ago

Hi @sitadrost

No worries! We will close the issue now and if the problem shows up again. You can re-open this.

sitadrost commented 4 years ago

Wait, no, please don't close this issue yet! I meant I couldn't reproduce this R-peak thing that I mentioned in my initial post, that sometimes it would help to use a different method to extract the R-peaks. The error with ecg_delineate still shows up, and quite persistently so.

I'll see if I can get you some anonymous signal that reproduces this, alright?

DominiqueMakowski commented 4 years ago

Yes please, it'd be easier to fix if we can reproduce the error ☺️

sitadrost commented 4 years ago

Would this work for you (see attached)? You should be able to load it using

import numpy as np
filename = <path-to-file>
with open(filename, 'rb') as fn:
    ecg = np.load(fn)

ECG_err.zip

P.S. The sampling rate of this signal is 100 Hz

sitadrost commented 4 years ago

Just to let you know: with the signal I attached in my previous post, it does help to use a different R-peaks list. The neurokit2 peaks algorithm returns 11 R-peaks in this case, and using these as input for ecg_delineate results in the error I mentioned in my initial post. However, if I use the R-peaks returned by some code of my own, I get 12 peaks, and using these as an input for ecg_delineate works just fine. I still haven't figured out what exactly goes wrong, but I thought this might be helpful for you as well.

For sake of completeness, both R-peaks vectors: R-peaks_neurokit = array([ 59, 127, 195, 266, 349, 472, 547, 603, 697, 761, 850]) R-peaks_homemade = array([ 4, 59, 127, 195, 266, 349, 472, 547, 603, 697, 761, 850])

sitadrost commented 4 years ago

Any progress?

DominiqueMakowski commented 4 years ago

Apologies, this might take us some time as we are currently busy with other things. In the meantime, if you manage to figure out the problems, you can contribute by making a pull request, we'll appreciate your help

sitadrost commented 4 years ago

Sure, no bother, take your time! I'll let you know if I find something useful

JanCBrammer commented 4 years ago

@sitadrost, please post a minimal, reproducible example that exactly reproduces the issue (using the ECG signal you posted earlier). Also, please include the full traceback (i.e., error message) in your example. Then it will be easier for us to help you.

sitadrost commented 4 years ago

For the full traceback see my initial post. Two examples to reproduce this error (after executing the code I posted earlier, to load the ECG signal), and a third one that doesn't give problems:

# The simplest one
import neurokit2 as nk
fs = 100
signals, info = nk.ecg_process(ecg, sampling_rate = fs)
# In three steps
import neurokit2 as nk
fs = 100
ecg_cleaned = nk.ecg_clean(ecg, sampling_rate = fs)
rpeaks_nk = (nk.ecg_peaks(ecg_cleaned, sampling_rate = fs)[1]['ECG_R_Peaks'])
_, pqst = nk.ecg_delineate(ecg_cleaned, rpeaks = rpeaks_nk, sampling_rate = fs)
# And after changing rpeaks the way I mentioned earlier, it works without problems
import neurokit2 as nk
fs = 100
ecg_cleaned = nk.ecg_clean(ecg, sampling_rate = fs)
rpeaks_homemade = array([ 4, 59, 127, 195, 266, 349, 472, 547, 603, 697, 761, 850])
_, pqst = nk.ecg_delineate(ecg_cleaned, rpeaks = rpeaks_homemade, sampling_rate = fs)
JanCBrammer commented 4 years ago

Thanks @sitadrost, I could replicate the issue. The NeuroKit2 dev branch now contains a fix (see 58ca43d79dd8f9c197c541501e4b2bc3f217bf44). If you haven't done so already you can clone the dev branch to have the fix available immediatly. Otherwise it will be available once we release version 0.0.41.

sitadrost commented 4 years ago

Great, thanks @JanCBrammer , you're fast! I've switched to the dev branch, updated the whole lot, and now I'm getting a slightly different error message:

test = nk.ecg_process(ecg, sampling_rate = 100)
Traceback (most recent call last):

  File "<ipython-input-5-69156fbd34c6>", line 1, in <module>
    test = nk.ecg_process(ecg, sampling_rate = 100)

  File "/home/sita/Documents/Python/NeuroKit/neurokit2/ecg/ecg_process.py", line 109, in ecg_process
    cardiac_phase = ecg_phase(ecg_cleaned=ecg_cleaned, rpeaks=rpeaks, delineate_info=delineate_info)

  File "/home/sita/Documents/Python/NeuroKit/neurokit2/ecg/ecg_phase.py", line 90, in ecg_phase
    atrial[ppeaks] = 1.0

IndexError: arrays used as indices must be of integer (or boolean) type

Is there something else I should update/change?

JanCBrammer commented 4 years ago

@sitadrost, your use-case uncovered another small bug in the codebase. You can pull dev again to get the fix (2f89b8d44c6624c9c625339e7a0852f988f0563e). Please keep reporting issues should you run into them. Thanks for your contribution!

sitadrost commented 4 years ago

Great, thanks a lot! I'll try the fix, and hopefully I won't bother you again for some time now.

ecatherina commented 3 years ago

I faced the same issue using ecg_delineate function for some signals. Here the traceback:

IndexError                                Traceback (most recent call last)
<ipython-input-23-4e0332ab4f4d> in second_lead_features(df_reports)
     23                 ecg_cleaned = nk.ecg_clean(ecg['Lead II'], sampling_rate = fs)
     24                 _, rpeaks = nk.ecg_peaks(ecg_cleaned, sampling_rate = fs)
---> 25                 signal_dwt, waves_dwt = nk.ecg_delineate(ecg_cleaned, rpeaks['ECG_R_Peaks'], sampling_rate=fs, method="dwt")
     26 
     27                 for start, end in zip(waves_dwt['ECG_P_Onsets'], waves_dwt['ECG_P_Offsets']):

~\Anaconda3\lib\site-packages\neurokit2\ecg\ecg_delineate.py in ecg_delineate(ecg_cleaned, rpeaks, sampling_rate, method, show, show_type, check)
    121         waves = _ecg_delineator_cwt(ecg_cleaned, rpeaks=rpeaks, sampling_rate=sampling_rate)
    122     elif method in ["dwt", "discrete wavelet transform"]:
--> 123         waves = _dwt_ecg_delineator(ecg_cleaned, rpeaks, sampling_rate=sampling_rate)
    124 
    125     else:

~\Anaconda3\lib\site-packages\neurokit2\ecg\ecg_delineate.py in _dwt_ecg_delineator(ecg, rpeaks, sampling_rate, analysis_sampling_rate)
    194     ponsets, poffsets = _dwt_delineate_tp_onsets_offsets(ppeaks, dwtmatr, sampling_rate=analysis_sampling_rate)
    195     tonsets, toffsets = _dwt_delineate_tp_onsets_offsets(
--> 196         tpeaks, dwtmatr, sampling_rate=analysis_sampling_rate, onset_weight=0.6, duration=0.6
    197     )
    198 

~\Anaconda3\lib\site-packages\neurokit2\ecg\ecg_delineate.py in _dwt_delineate_tp_onsets_offsets(peaks, dwtmatr, sampling_rate, duration, duration_offset, onset_weight, offset_weight, degree_onset, degree_offset)
    370             continue
    371         candidate_offsets = np.where(-dwt_local[offset_slope_peaks[0] :] < epsilon_offset)[0] + offset_slope_peaks[0]
--> 372         offsets.append(candidate_offsets[0] + srch_idx_start)
    373 
    374         # # only for debugging

IndexError: index 0 is out of bounds for axis 0 with size 0

Here is my code

ecg_signal = np.load(os.path.join(dirpath, file))                
fs = 500
ecg_cleaned = nk.ecg_clean(ecg_signal, sampling_rate = fs)
_, rpeaks = nk.ecg_peaks(ecg_cleaned, sampling_rate = fs)
signal_dwt, waves_dwt = nk.ecg_delineate(ecg_cleaned, rpeaks['ECG_R_Peaks'], sampling_rate=fs, method="dwt")

plot = nk.events_plot(rpeaks['ECG_R_Peaks'], ecg_cleaned)

I tried to visualise detected R peaks and they seem to be wrong detected. One peak is undetected and one is extra detected.

image

My System Specifications

OS: Windows 10 Python: 3.7.4 NeuroKit2: 0.0.42

Would appreciate any help!

DominiqueMakowski commented 3 years ago

@ecatherina thanks for the feedback. Could you share your version of NK? (nk.version())

ecatherina commented 3 years ago

@DominiqueMakowski, sure, thank you for the reply!

- OS: Windows (WindowsPE 64bit) 
- Python: 3.7.4 
- NeuroKit2: 0.0.42 

- NumPy: 1.16.5 
- Pandas: 1.1.3 
- SciPy: 1.4.1 
- sklearn: 0.22.1 
- matplotlib: 3.1.3
JanCBrammer commented 3 years ago

@DominiqueMakowski, I think we should consider taking a closer look at ecg_delineate top to bottom (see also #387). Looking at the module with a linter it's apparent that there are a number of unbound variables, unused variables, as well as missing checks for emptiness (resulting in the IndexError reported here).

DominiqueMakowski commented 3 years ago

Yes true, and possibly clean and refactor its implementation to divide in several subparts, so that it's more modular and robust. Tagging @Tam-Pham I know she'll love jumping back into that 😬

Tam-Pham commented 3 years ago

Alright. Lemme have a look as soon as I can.

JanCBrammer commented 3 years ago

@Tam-Pham, let me know if and how I can help (divide and conquer).

DominiqueMakowski commented 3 years ago

I feel like maybe @JanCBrammer you could indeed try to give it a fresh look first, which might help identify weird/unclear/suspicious/poorly-implemented parts (sometimes it's easier to find what's wrong in others' code than your own 😁) and then we can help clean and improve the function!

JanCBrammer commented 3 years ago

Will do, should have time to take a look at this within the next 3 weeks.

stale[bot] commented 3 years ago

This issue has been automatically marked as inactive because it has not had recent activity. It will eventually be closed if no further activity occurs.

DominiqueMakowski commented 3 years ago

@Tam-Pham @zen-juen did we fix / improve this or is it still to do?

Tam-Pham commented 3 years ago

I remember I did went through the functions to add in more checks for empty lists, and thus to avoid such index error. But it seems like I didnt reference the changes to this issue. Do leave it as to-do still

stale[bot] commented 2 years ago

This issue has been automatically marked as inactive because it has not had recent activity. It will eventually be closed if no further activity occurs.

stale[bot] commented 2 years ago

This issue has been inactive for a long time. We're closing it (but feel free to reopen it if need be).