RocketPy-Team
ENH: Add STFT function to Function class
Pull request type
- [x] Code changes (bugfix, features)
Checklist
- [ ] Tests for the changes have been added (if needed)
- [ ] Lint (
black rocketpy/ tests/) has passed locally
Current behavior
STFT feature was requested in issue #308 with link to #292 as example
New behavior
Added STFT
@AdvaitChandorkar07 thank you so much for your contribution! We are going to review your PR as soon as possible.
At a first glance, I think it solves the problem as requested by #308 . Good job!
Thank you for your contribution, @AdvaitChandorkar07, and sorry for the long delay for a review.
The code itself is well-written and clear. For the actual results, we need a more suitable example to compare the
implementation. I compared the results of your implementation with SciPy's SFTF example. I had to adapt it to use the boxcar window function to match your implementation. Here are the results of the spectrograms:
The results look very similar, although there were some minor differences (check the colors around 25s and at the beginning and end of the signal). One major difference is that the the SciPy FFT magnitude seems to be half of the FFT magnitude computed by RocketPy. Here is the very simple notebook I used to compare SciPy to your implementation. testing_stft.zip
Overall, things look very good. Some final comments:
(1) We should investigate why the FFT magnitude of SciPy seems to be half of our implementation. Maybe I did something wrong in the examples or computing the absolute values?
(2) Do you have an interest in implementing different window functions? For instance, the SciPy STFT win argument is another function that is multiplied by the signal in each window. However, this would make the implementation more complicated, so I think it's okay to leave this as a feature for the future;
(3) Adapting the example as was mentioned in the review would help a lot in understanding how useful the STFT is.
Thanks a lot for the contribution, @AdvaitChandorkar07 !! Great work.