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

Time.py using IFFT instead of FFT to transform from time domain to frequency domain #390

Closed
RA226 opened this issue Sep 11, 2020 · 3 comments
Labels
Is it a bug? To be confirmed if it is a bug or not

Comments

@RA226
Copy link

RA226 commented Sep 11, 2020

Hi,
I have been using sckit-rf to successfully time gate calibrated data from a quasi-optic system (50-750GHz).

I noticed today on line 266 of Time.py:

265 #IFFT the gate, so we have it's frequency response, aka kernel
266 kernel=fft.fftshift(fft.ifft(fft.ifftshift(gate, axes=0), axis=0))
267 kernel =abs(kernel).flatten() # take mag and flatten
268 kernel=kernel/sum(kernel) # normalize kernel

An inverse Fast Fourier transform is being used to transform from the time domain to the frequency domain. This should be a Fast Fourier Transform and not the inverse.

I have tested this change and it makes no difference to the results especially as only the magnitude is used and the kernel is normalized on the following line. In that sense it is not a bug but is incorrect!

Regards

Roger Appleby

@jhillairet jhillairet added the Is it a bug? To be confirmed if it is a bug or not label Sep 11, 2020
@jhillairet
Copy link
Member

@RA226 wasn't another problem the fact you used negative time as start time?

@RA226
Copy link
Author

RA226 commented Sep 11, 2020

Hi,
I checked the latest version of the code and it works fine with negative start time and positive end time. Therefore I did not include this as an issue.

I can see looking at the commit for 221 97ac281 that ifftshift was corrected and the sign changed from - to + . I can confirm that + works fine in my case. I dont believe that I was using scikit-rf before this commit so it has always been this way during the time I have been using it.

Let me know if I can help further.
Regards
Roger Appleby

@jhillairet
Copy link
Member

issue resolved from #391

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Is it a bug? To be confirmed if it is a bug or not
Projects
None yet
Development

No branches or pull requests

2 participants