paulvangentcom / heartrate_analysis_python

Python Heart Rate Analysis Package, for both PPG and ECG signals
MIT License
930 stars 321 forks source link

Added condition to 'calc_fd_measures' function to prevent crash #61

Closed Arritmic closed 3 years ago

Arritmic commented 3 years ago
Arritmic commented 3 years ago

Yes, finally after messing a little bit with the CI configuration, it looks like it is working properly and it passes all the tests.

Cheers

paulvangentcom commented 3 years ago

Thanks Arritmic.

While I'm in favour of fixing crash conditions, I'm not sure silently passing on here is the right strategy. With frequency metrics 3 RR intervals or less is way too little, your frequency metrics will be essentially meaningless.

Let me think a bit on what would be a correct strategy here. I'm leaning towards throwing an exception because I don't want to give the impression the outputs are valid in these conditions.

-Paul

Arritmic commented 3 years ago

Hi Paul, I fully align with your thoughts. I don't remember well the values I was obtaining with this "patch", but it seemed coherent according to the results in the previous and next windows. Anyway, I will reproduce the "error" again in the evening, and I will provide you more details. The error happens only once for the baseline data of the subject S2 of the database, and that BVP time series is about 70k samples.

A possible solution could be to return a NaN number, so the person who gets the results can decide how to manage that values in the middle of sequence of results. In my case, I can fill it with the same value of the previous window in that sequence.

S2_BVP_signal

paulvangentcom commented 3 years ago

Thanks for the input Tino. The error reproduced will help me see what's going on too.

I like your NaN suggestion. Perhaps with a flag to bypass this and still produce data, at least then people will surely understand why they're turning it on in the first place...

What do you think?

Cheers

Arritmic commented 3 years ago

Yes, that seems reasonable and a solution. Probably it is a rare exception, but I will be working with your framework during next months. If I find something else, I will let you know it. I am just uploading a possible solution since this part it is only related to the measurement of the energy of the windows mainly. I was checking the results, and it generates a lot of NaN numbers already when the rr_list is little. See attached image... (In the screenshot, it is working the solution of decreasing the degree, but you can observe that already in the previous windows with short rr_list the most of the parameters are NaN).

Other possible solution it could be to try to compute the energy using time domain approximations (Parseval's theorem), for giving always the energy of the signal.

Screenshot from 2021-03-15 19-03-53

paulvangentcom commented 3 years ago

Thanks for attaching the picture. What likely happens is that some HRV metrics such as RMSSD and SDSD require at least 3 consecutive beats to be computed, meaning two RR-intervals with no rejected peaks in between or large distance apart. In these cases of few RR-intervals the NaNs here are intentional behaviour.

For the frequency metrics I'll read up a bit on that and the accuracy of those methods. With so few RR-intervals, of which the odds are they are not adjacent, frequency metrics will make little sense to compute. For those where 3 or 4 RR-intervals do align, you're estimating frequency metrics on (much) less than a full period of the signal. I'll look into whether we can with some confidence compute results that at least mean something.

Thanks again, and feel free to ask if anything else pops up in your usage

Cheers, Paul

Arritmic commented 3 years ago

Thanks to you for the effort and the library. :) Glad to be useful. BR, Constantino