search

ik1xpv / ExtIO_sddc

ExtIO_sddc.dll - BreadBoard RF103 / HDSDR
Other
73 stars 26 forks source link

Increase NDECIDX to 16 #145

Closed howard0su closed 3 years ago

howard0su commented 3 years ago

Sharing the fftw plan across the worker threads to avoid too many plans when NDECINDEX is large. Prepare to support more sample rates.

hayguen commented 3 years ago

http://www.fftw.org/fftw3_doc/Thread-safety.html#Thread-safety writes

since the plan is not modified by fftw_execute, it is safe to execute the same plan in parallel by multiple threads

but also

the only thread-safe routine in FFTW is fftw_execute (and the new-array variants thereof)

suppose, this is granted

hayguen commented 3 years ago

ok, these changes run/look fine .. without having made detailed measurments. tested reception on VHF. as this is only the preparation for more samplerates .. the ExtIO doesn't provide additional samplerates .. i think, i need to measure stopband attenuation with a signal generator and a sweep signal on HF - for the Kaiser window and -120dB ..

howard0su commented 3 years ago

I didn't modify EXTIO part to report more sample rate. will do that after this merged.

hayguen commented 3 years ago

i would suggest to merge these first changes onto a different branch - not master, that we all can work together on this stuff. i fear, that we really need to add dynamic filter length, that the additional samplerates get usable

howard0su commented 3 years ago

Sure. let me create one.

howard0su commented 3 years ago

http://www.fftw.org/fftw3_doc/Thread-safety.html#Thread-safety writes

since the plan is not modified by fftw_execute, it is safe to execute the same plan in parallel by multiple threads

but also

the only thread-safe routine in FFTW is fftw_execute (and the new-array variants thereof)

suppose, this is granted

Yes. I considered that.

ik1xpv commented 3 years ago

i would suggest to merge these first changes onto a different branch - not master, that we all can work together on this stuff. i fear, that we really need to add dynamic filter length, that the additional samplerates get usable

referring to wikipedia It seems that the solution is to increase the fft dimension too, By the way we use FFTN/4 +1 ht length. that seems the double of what picture shows as limit :-) May be a two stage decimation be more convenient ?

hayguen commented 3 years ago

you see my KaiserWindow() debug/test results above ..

@howard0su / @ik1xpv : please create the branch .. and we change this pull request's target branch .. and merge this pull request there. give it same name as Howard?

after that, i can commit/pull my minor changes in KaiserWindow() and the above debug output .. and we can play around together

hayguen commented 3 years ago

referring to wikipedia It seems that the solution is to increase the fft dimension too,

i would try to keep the fft size low .. that CPU's data cache is effectively used. looking at first graph of https://github.com/hayguen/pffft_benchmarks/blob/master/i5-3320M_gcc-9.2.0_x64_Gentoo-2.6.md , fft size of 1024 or 2048 looks to be optimum on an i5. looking at https://github.com/hayguen/pffft_benchmarks/blob/master/i7-4790_gcc-7.5.0_LinuxMint19.3.md , there's not much changed on an i7.

By the way we use FFTN/4 +1 ht length. that seems the double of what picture shows as limit :-)

that pretty much depends on their cost function. they just consider the number of multiplications. nowadays, we must consider cache/memory also!

May be a two stage decimation be more convenient ?

i'd prefer keeping it simple with single stage .. and revise the target

hayguen commented 3 years ago

@howard0su / @ik1xpv : please create the branch .. and we change this pull request's target branch .. and merge this pull request there. give it same name as Howard?

created branch 'ndec' and changed this pull requests target branch myself .. now one of should accept this pull request ..

howard0su commented 3 years ago

thanks. I will close this PR.

On Fri, Jan 8, 2021 at 8:51 PM hayati ayguen notifications@github.com wrote:

@howard0su https://github.com/howard0su / @ik1xpv https://github.com/ik1xpv : please create the branch .. and we change this pull request's target branch .. and merge this pull request there. give it same name as Howard?

created branch 'ndec' and changed this pull requests target branch myself .. now one of should accept this pull request ..

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ik1xpv/ExtIO_sddc/pull/145#issuecomment-756739767, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAF3GRELACHRTOJDWSOJEMDSY35U5ANCNFSM4VZZ6ONA .

-- -Howard

hayguen commented 3 years ago

why close without merge?

howard0su commented 3 years ago

you created ndec branch. I assume that you want us to work on that branch and submit PR from there.

On Fri, Jan 8, 2021 at 9:40 PM hayati ayguen notifications@github.com wrote:

why close without merge?

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/ik1xpv/ExtIO_sddc/pull/145#issuecomment-756760550, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAF3GRAB7MVXCTPDFLJ6YHLSY4DL3ANCNFSM4VZZ6ONA .

-- -Howard

hayguen commented 3 years ago

that created branch is/was empty .. and was just to allow changig this pull requests target branch on github. i accept this pull request ..