NeuroKit icon indicating copy to clipboard operation
NeuroKit copied to clipboard

Zhao2018 quality estimate function has wrong window length defaults

Open Miceuz opened this issue 3 years ago • 5 comments

Describe the bug In documentation it is written

The "zhao2018" method (Zhao et la., 2018) was originally designed for signal with a length of 10 seconds.

However default value for window is 1024: def _ecg_quality_zhao2018(ecg_cleaned, rpeaks=None, sampling_rate=1000, window=1024, kurtosis_method="fisher", mode="simple", **kwargs)

While stating that window argument is in seconds, not milliseconds.

window : int Length of each window in seconds. See signal_psd().

Also signal_psd stated that window is in seconds:

window : int Length of each window in seconds (for Welch method). If None (default), window will be automatically calculated to capture at least 2 cycles of min_frequency. If the length of recording does not allow the formal, window will be default to half of the length of recording.

Miceuz avatar Jun 17 '21 16:06 Miceuz

Hi there @Miceuz thanks for raising this issue! Let me take a look at the original paper that implemented this algorithm and get back to you 😄

zen-juen avatar Jun 18 '21 01:06 zen-juen

Hmm looking into the database that the original authors used (they used two databases from the Physionet/Cinc Challenge 2011 and Physionet/Cinc Challenge 2017), it doesn't seem like the signals used were 10 seconds in length. For example, the 2017 database contains signals obtained from "a single short ECG lead recording (between 30 s and 60 s in length)" so I'm not too sure why we documented that the algorithm was designed for specifically a 10s-long signal 🤔 I might be missing something here but it seems to me that this algorithm doesn't have to be used for only 10s signals (if @DominiqueMakowski can confirm 😄 we can then correct this documentation)

Regarding the window argument which defaults to 1024 (which relates to the segments of samples used) this is an internal function specifically to calculate the power spectrum density (used in nk.signal_psd()) which is then passed to calculate the quality of the signal - so this has nothing to do with the length of the ECG signal. Hope this clarifies!

zen-juen avatar Jun 18 '21 03:06 zen-juen

I think we took the description from biosppy's without double checking it, but yeah if it's not correct then we need to change. Thanks for bringing our attention on this @Miceuz

DominiqueMakowski avatar Jun 18 '21 04:06 DominiqueMakowski

My original idea when posting this bug report was that _ecg_quality_zhao2018 assumes window is 1024 while from user-facing API it's not clear what is the minimum length the recording has to be. In fact at the bottom signal_psd requires the signal to be at least 1024, call would fail with smaller signal sizes.

Also, judging from signal_psd documentation window is in seconds, not in samples.

Miceuz avatar Aug 02 '21 10:08 Miceuz

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 Apr 18 '22 18:04 stale[bot]

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

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

stale[bot] avatar Sep 08 '22 17:09 stale[bot]