Fix matrix B created too short

[pjercic]

Description

This PR aims at adding this fix where the data matrix B for creating the sparse matrix D_2 was created too short (N-2) and it didn't reach the two last columns in the sparse matrix upon creation.

Proposed Changes

I changed the _signal_detrend_tarvainen2002(signal, regularization=500) function so that I extended the matrix B to fix the mentioned issue.

Checklist

Here are some things to check before creating the PR. If you encounter any issues, do let us know :)

• [x] I have read the CONTRIBUTING file.
• [x] My PR is targetted at the dev branch (and not towards the master branch).
• [x] 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[bot]]

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

[DominiqueMakowski]

Hi @pjercic thanks for this PR.

I am not sure about that fix since this is an existing algorithm which is implemented in a similar way elsewhere; e.g.:

• https://github.com/rhenanbartels/hrv/blob/190c923250884b1f5632e38658bd8df7d9d5352e/hrv/detrend.py#L115
• https://github.com/LucaCerina/HRV-detrend/blob/master/detrendFast.m#L32

I don't remember the original paper, but if you could check that source that'd be great, so that we know whether it is the implementations that are possibly wrong, or perhaps the original algo is bug-prone too.

In what context did you obtain an error?

[JanCBrammer]

The algorithm seems to be published here: https://sci-hub.st/10.1109/10.979357

Tarvainen, M. P., Ranta-aho, P. O., & Karjalainen, P. A. (2002). An advanced detrending method with application to HRV analysis. IEEE Transactions on Biomedical Engineering, 49(2), 172–175. doi:10.1109/10.979357

[DominiqueMakowski]

Thanks @JanCBrammer, so we shouldn't touch it in your opinion?

[JanCBrammer]

I think that depends on whether or not the original reference is correct. @pjercic, the bug might as well originate there?

[pjercic]

I hit an error using your code to do smooth priors de-trending, and pinpointed it to the issue of the generator data for creating matrix being too short. Therefore I fixed it and it also fixed the output problem specified here.

https://stackoverflow.com/questions/69810723/issues-with-signal-detending-using-smoothness-priors-on-the-last-detended-elemen

I am not sure where the error ended up in your pipeline with calling library errors, but the real failed case is of course, and assert values, since the output of this specific de-trend was not correct. FAILED tests/tests_signal.py::test_signal_detrend - assert False

[DominiqueMakowski]

Pinging @rhenanbartels for his opinion about that -2 since we currently share the same implementation :)

[pjercic]

There have been further developments where another person suggested a further improvement. I need to test on my side, but it is a sound proposal, check it out.

https://stackoverflow.com/questions/69810723/issues-with-signal-detending-using-smoothness-priors-on-the-last-detended-elemen/74365247#74365247