Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validating the output #4

Closed
danigb opened this issue Jan 19, 2017 · 11 comments
Closed

Validating the output #4

danigb opened this issue Jan 19, 2017 · 11 comments

Comments

@danigb
Copy link
Contributor

danigb commented Jan 19, 2017

Hi,

I suppose I'm neither the first of or the last that, frustrated by the abandon of dsp.js library, tries to maintain an updated version of the bests parts. So, I've started a port of rsft (before discover your project): https://github.com/oramics/dsp-kit/tree/master/packages/rfft

I also did my own benchmarks (at least repeating is good for learn ;-). Finally I've tried to include the inverse rfft implementation: https://github.com/oramics/dsp-kit/blob/master/packages/rfft/lib/inverse.js

But the problem is that I discover that this rfft implementation returns a result that is completely different from the fftimplementation of the same dsp.js library. The test is here: https://github.com/oramics/dsp-kit/blob/master/packages/rfft/test/test.js#L7

Have you tested the output? Are you aware of this problem? Did you used this implementation in any project?

Thanks in advance.

@dy
Copy link
Member

dy commented Jan 19, 2017

Hi @danigb!

I see your concern in fft-s here and there.

Yeah, I tested out this rfft implementation in benchmarks and across several other projects (like here), and I can say the results were the same as ndarray-fft and analyserNode's fft. The only problem I noticed is negative static level, which was mentioned in the issue.

You can show the code if you face any troubles with setting that up.

@danigb
Copy link
Contributor Author

danigb commented Jan 19, 2017

Yes... I'm still a bit concern about it. I did my own ultra-slow (but I hope correct) DFT implementation and run the tests against it. It seems to work with dsp.js FFT but not with RFFT.

Also I have another problem with this implementation: I'm writing a phase vocoder time stretch algorithm and I need, not only the magnitude spectrum, but the phases and an inverse rfft operation. Have you consider the idea of:

  1. Add the phase or return the result in rectangular coordinates?
  2. Implement the inverse RFFT (there is an attempt here, as you probably know: Rfft inverse transform implemented corbanbrook/dsp.js#5)

Thanks @dfcreative


EDIT: I'll do more tests, anyway...

@dy
Copy link
Member

dy commented Jan 20, 2017

Ok.

  1. Phase can be inferred from Re/Im parts of FFT, for now only real parts are implemented, that is concern of this issue.
  2. Inverse FFT I believe should behave the same as direct FFT (wiki), but I didn't test that practically. I tried to do synthesis via IDFT here, but did not manage to do phase matching, going to finish that soon.

@danigb
Copy link
Contributor Author

danigb commented Jan 20, 2017

Hi!

  1. Yes, but the Re/Im parts are discarded (https://github.com/scijs/fourier-transform/blob/master/index.js#L182). Some changes to the code should be necessary, including working with arrays, with performance implications, so it require carefully thinking.
  2. Interesting approach, I will take a look for sure. Your synthesis thing concept looks quite similar to the phase vocoder I'm trying to implement: https://github.com/oramics/dsp-kit/tree/master/packages/phase-vocoder

I'll try to do some test soon, and some research about the inverse transform.

Regards,
Dani

@danigb
Copy link
Contributor Author

danigb commented Jan 22, 2017

Hi @dfcreative

First of all I have to say that, after dive into your code, I got really impressed with the benchmark suite and the number of libraries you tested. Congrats.

Second, I realized that with this implementation I can not do inverse FFT since the inverse FFT must use the real and the imaginary part.

But most important, I was still suspecting about the differences of the output and I realize that the test has a bug that doesn't perform the comparation between the results. In line 30: https://github.com/scijs/fourier-transform/blob/master/test.js#L30 a return is missing, so it only checks the first element of each array. If you add a return statement you will see the test failing :-(
... I'm afraid fft and rfft outputs are quite different, but I have to test it more carefully.

@dy
Copy link
Member

dy commented Jan 23, 2017

@danigb thanks for the awesome point!

Indeed I see that difference. That is less noticeable though if to look at data renders

ft

First is RFFT, second is dsp.FFT and the third is ndarray-fft. Seems that precision is [deliberately?] lost for the dsp case.

@danigb
Copy link
Contributor Author

danigb commented Jan 24, 2017

Hi @dfcreative

This graphs are an eye opener for me! The difference is much less that I thought (by looking at the numbers! 😂 ). So, do you think the differences in precision are not a big deal for normal fft use, isn't it?

@dy
Copy link
Member

dy commented Jan 24, 2017

@danigb well for rendering purpose RFFT is completely fine I believe, mb even for analytical purposes as well.
As for resynthesis I am not that sure, anyways it takes STFT with frame size desirably more than 4096 samples, which takes some additional implementation details.
I think #3 and #2 should provide alternative FFT methods, seemingly with better precision.

@rreusser
Copy link
Member

rreusser commented Jan 30, 2017

Is the issue in the three graphs the horizontal alignment of the peak? Just a total guess but the most difficult part of FFT for me is always just interpreting the data. It looks to me like one of those off-by-one indexing issues. (was it floor((n + 1)/2)… ceiling? divide by n? n-1? that sort of thing). Just a guess.

@dy
Copy link
Member

dy commented Jan 30, 2017

@rreusser yes, that is the thing with ndarray-fft, but the actual issue is tiny difference in peak magnitude of the first and the second graphs. The origin of the difference may be the same - some indexing is off by one. Or possibly discarding complex part, etc.

@danigb
Copy link
Contributor Author

danigb commented Jan 31, 2017

Hi @rreusser, thanks for joining the conversation. Indeed, the n or n-1 problem was a mistake a made several times. Anyway, my confidence with the different fft implementations are slowly increasing ;-)

@dy dy closed this as completed May 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants