added spectrogram #23
Conversation
|
Hi Ben, this is definitely something we need and I also did not find anything similar in any published package. We should have some tests for this function before adding it to the package. I'll try to come up with something, but feel free to propose some tests too. |
| p=hcat(p...) | ||
| t=[1:size(p,2)]*(nfft-noverlap)/fs | ||
| f=[1:size(p,1)]/size(p,1)*(fs/2) |
There was a problem hiding this comment.
Why not return ranges (replace the brackets with parens) instead of arrays?
|
This is another case where there is substantial performance to be gained by using the FFTW plan interface, but that interface is currently in flux (see JuliaLang/julia#6193), so I'm fine with merging this for now. I personally appreciate the utility of accepting the sample rate and returning frequencies here, but if we do it, we should probably also do it for the other functions in this module. |
|
returning ranges and making tests are both great ideas. for the latter, how about testing its output with matlab? should probably add docs too. |
|
Comparing with output from another tool (MATLAB/Python) is a good idea, but in this case you should export the output to a text file instead of generating it on-the-fly (that way, we don't have additional requirements for testing the package). Regarding documentation, feel free to add it to the file corresponding to the periodogram module in the doc/ directory. The website is automatically updated after each commit to master. |
|
docs, tests, ranges, and bug fixes all now committed. i also changed the input arg names to be consistent with the rest of the module. i was unable to figure out how to run the tests automatically, but it all works when done by hand. |
|
this new commit is what i have in mind for inputing a window function to spectrogram. also note that filter_design's tests are failing. i don't think this is my fault. i did however fix some deprecations with this commit. |
|
I noticed our filter_design tests are failing, too. There are some wrong coefficients in the computed filters. @simonster, could you take a look as soon as you have some time? Thanks! I am a bit busy currently and there's no hope of it getting better for the next couple of weeks, so I still could not test everything and merge your updates, sorry! |
|
It seemed like the differences in the filter coefficients were still quite small (< eps, apparently) so I just changed the error bound. I should probably track down exactly when/why this started failing, though. There was also an odd bug in the Hilbert tests apparently due to |
|
should've tested with non-unitary sampling rates in the first couple of commits! |
|
Sorry, I completely forgot about this PR. Merged, and thanks! |
please consider adding this convenience function. i couldn't find it anywhere in julia or it's packages. let me know if i missed it!