Stanford-STAGES / stanford-stages

Automated sleep staging scoring and narcolepsy identification
76 stars 26 forks source link

Small differences in encoding results between original MATLAB implementation and current Python implementation #24

Closed informaton closed 3 years ago

informaton commented 4 years ago
informaton commented 4 years ago

Differences between padding lengths between MATLAB and scipy's filfilt function were address using MATLAB's default. See this post for more info: https://dsp.stackexchange.com/questions/11466/differences-between-python-and-matlab-filtfilt-function

informaton commented 4 years ago

I am looking into the updates now and will compare results before merging into the master branch.

informaton commented 4 years ago

I updated code in the _extracthjorth method

in order to match the original MATLAB call (i.e. buffer(input, dim, dim-slide, 'nodelay')) from __load_signal.m__. I also removed the follow-on D = np.delete(D, -1, axis=-1) (in _extracthjorth) as this looks to be addressed already in the rolling_window_nodelay method. Let me know if I have that correct @neergaard.

informaton commented 4 years ago

I updated the rolling_window_nodelay method as the calculate_padding method did not always give the correct number of zeros to pad with compared to the output of MATLAB's buffer(...,'nodelay') call. There are some comments in the code annotating the changes and giving an example for the needed change.

neergaard commented 4 years ago

@informaton No, I'm not sure that's correct. If I read the arguments to buffer correctly, the third argument is the amount of overlap, which is the 'inverse' of the step argument from rolling_window_nodelay, The extract_hjorth method is not calculating the parameters over overlapping segments (third argument is dim - slide, but they are both 5 * 60 * fs), although we could do that in the future of course.

But I think it's fine to delete the D = np.delete part.

neergaard commented 4 years ago

Wait, I just reread the matlab implementation of the extract_hjorth method, and the np.delete part is actually in there as well: if you look here on line 174: https://github.com/Stanford-STAGES/sleep-staging/blob/dev/matlab_training_data_scripts/load_signal.m

So it should stay :)

informaton commented 4 years ago

@neergaard That's how I read the buffer instructions as well, that the third argument is the 'inverse' of the step argument. I'll update it to

D = rolling_window_nodelay(x, dim, slide)

in order to keep the option to add overlap going forward and not cause confusion with the slide input parameter. Good catch on the delete of the last column from the load_signal.m file too. I'll add that back in as well. Thanks!

Also, the additional checks in the rolling_window_nodelay accounted for when calculate_padding differences created an extra column so the final results were consistent with the MATLAB buffer method. So the changes to weren't needed per se but I wanted to simplify that method and remove the extra math import if possible.

informaton commented 4 years ago

@neergaard I added you as a coauthor on the last commit (2b48ff7). Feel free to close the issue if it looks good to you, or add more comments as necessary.