telegraphic / pfb_introduction

An interactive introduction to the polyphase filterbank technique for radio astronomy spectrometers
51 stars 16 forks source link

Test using continuous uniform pdf instead of Gaussian distribution pdf #4

Closed xbosch closed 4 years ago

xbosch commented 4 years ago

First of all good job with the code. I ported to Matlab and I could not find any error or problem.

The only issue that I have seen is when you test the PFB you use np.random.random() to generate noise. np.random.random has a uniform probability density function (pdf), according to the Numpy webpage:

Results are from the “continuous uniform” distribution over the stated interval. [0,1] by defeault

Thermal noise, which is what you want to simulate to emulate RF noise, has a Gaussian pdf. To simulate Gaussian noise you should use np.random.randn() instead, according to the Numpy webpage:

randn generates an array of shape (d0, d1, ..., dn), filled with random floats sampled from a univariate “normal” (Gaussian) distribution of mean 0 and variance 1

This code issue is in both the .py and the jupyter notebook.

Best Wishes!

telegraphic commented 4 years ago

Hey @xbosch, thanks for checking it out, glad you found it useful! Agreed that Gaussian noise should be used, will update accordingly.

By the way, please feel free to push your MATLAB code if you're interested (repo license is CC-BY 4)

texadactyl commented 4 years ago

@telegraphic Fixed in PR #6. I used a numpy normal distribution function (random.normal) which has more parameter options. My guess is that you still wanted the distribution centered around 0.5 spread from 0 to 1 to get the desired noise values.

telegraphic commented 4 years ago

Hi @texadactyl, fancy seeing you here! Thanks, merged.

texadactyl commented 4 years ago

I try to never miss a chance to learn something new or relearn in a new way.

Looking at Julia, its easy to convert first-hand code; even numpy functions are built-in (how efficient?). It is quite another matter to identify which libraries to use in place of scipy. Probably Julia's "DSP".

xbosch commented 4 years ago

Guys, I have the codes in matlab, I can send them to you if you are interested XB

On Sun, Sep 20, 2020 at 6:24 AM Richard Elkins notifications@github.com wrote:

I try to never miss a chance to learn something new or relearn in a new way.

Looking at Julia, its easy to convert first-hand code; even numpy functions are built-in (how efficient?). It is quite another matter to identify which libraries to use in place of scipy. Probably Julia's "DSP".

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/telegraphic/pfb_introduction/issues/4#issuecomment-695786930, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABBGKYFEW7CAWZPOJRT4EQTSGX675ANCNFSM4L3F5BZA .

-- Se'n va la primavera. Ploren les aus i son llàgrimes els ulls dels peixos.

Matsuo Bashö

texadactyl commented 4 years ago

@xbosch It was simple to fix.

xbosch commented 4 years ago

Hi,

That is right. It is not clear what is the fix from the title. Thermal noise is Gaussian, therefore the overall PFB test should be done in Gaussian noise, but the title suggests the other way around. Unrelated to the fix: I was saying that I implemented the same code in Matalb in case you would like to add it to the repository.

Best, XB

On Mon, Sep 21, 2020 at 10:51 AM Richard Elkins notifications@github.com wrote:

@xbosch https://github.com/xbosch It was simple to fix.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/telegraphic/pfb_introduction/issues/4#issuecomment-696270844, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABBGKYFKLR35IQDIYWU7N23SG6HDVANCNFSM4L3F5BZA .

-- Se'n va la primavera. Ploren les aus i son llàgrimes els ulls dels peixos.

Matsuo Bashö

texadactyl commented 4 years ago

@xbosch Yes, Danny would be interested as he already inidcated. I didn't mean to discourage you. Feel free to open a PR with your MATLAB source code added.

telegraphic commented 4 years ago

@xbosch yes still v. happy to add MATLAB code!