NeuroKit icon indicating copy to clipboard operation
NeuroKit copied to clipboard

Function ecg_delineate gives IndexError

Open sitadrost opened this issue 3 years ago β€’ 30 comments

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

  • OS: Linux ( 64bit)

  • Python: 3.7.7

  • NeuroKit2: 0.0.40

  • NumPy: 1.18.5

  • Pandas: 1.0.5

  • SciPy: 1.5.0

  • sklearn: 0.23.1

  • matplotlib: 3.1.3

sitadrost avatar Sep 15 '20 06:09 sitadrost

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!

Tam-Pham avatar Sep 15 '20 07:09 Tam-Pham

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_delineate is called as a part of ecg_process, and gives the error that I showed in my initial post.

sitadrost avatar Sep 15 '20 08:09 sitadrost

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!

sitadrost avatar Sep 15 '20 08:09 sitadrost

Hi @sitadrost

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

Tam-Pham avatar Sep 15 '20 09:09 Tam-Pham

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?

sitadrost avatar Sep 15 '20 09:09 sitadrost

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

DominiqueMakowski avatar Sep 15 '20 09:09 DominiqueMakowski

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 avatar Sep 15 '20 10:09 sitadrost

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 avatar Sep 16 '20 14:09 sitadrost

Any progress?

sitadrost avatar Sep 29 '20 08:09 sitadrost

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

DominiqueMakowski avatar Sep 29 '20 08:09 DominiqueMakowski

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

sitadrost avatar Sep 29 '20 09:09 sitadrost

@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.

JanCBrammer avatar Oct 01 '20 15:10 JanCBrammer

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)

sitadrost avatar Oct 02 '20 06:10 sitadrost

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.

JanCBrammer avatar Oct 02 '20 07:10 JanCBrammer

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?

sitadrost avatar Oct 02 '20 09:10 sitadrost

@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!

JanCBrammer avatar Oct 03 '20 13:10 JanCBrammer

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

sitadrost avatar Oct 03 '20 14:10 sitadrost

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!

ecatherina avatar Jan 12 '21 10:01 ecatherina

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

DominiqueMakowski avatar Jan 12 '21 11:01 DominiqueMakowski

@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

ecatherina avatar Jan 12 '21 12:01 ecatherina

@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).

JanCBrammer avatar Jan 12 '21 12:01 JanCBrammer

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 😬

DominiqueMakowski avatar Jan 12 '21 12:01 DominiqueMakowski

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

Tam-Pham avatar Jan 12 '21 12:01 Tam-Pham

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

JanCBrammer avatar Jan 12 '21 12:01 JanCBrammer

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!

DominiqueMakowski avatar Jan 14 '21 03:01 DominiqueMakowski

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

JanCBrammer avatar Jan 18 '21 16:01 JanCBrammer

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] avatar Jul 17 '21 16:07 stale[bot]

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

DominiqueMakowski avatar Aug 20 '21 11:08 DominiqueMakowski

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

Tam-Pham avatar Aug 20 '21 15:08 Tam-Pham

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] avatar Feb 16 '22 19:02 stale[bot]