jgaeddert / liquid-dsp

digital signal processing library for software-defined radios
http://liquidsdr.org
MIT License
1.87k stars 439 forks source link

liquid_gcd() behaviour #315

Closed vvsvvsvvs closed 1 year ago

vvsvvsvvs commented 1 year ago

I do not know about the idea of the function liquid_gcd() (hope, it is GCD calculations) but calculated GCD for 50 vs 30 is 10 (OK), while for 55 vs 33 is 1 (OK, but it is not GCD, which is 11). Functions using the algo are OK (filters) due to the nature of calculations. But the fact will lead to longer loops with longer filter banks that are not really used. Maybe, it is better to use Euclidean way, like:

liq_gcd(int q, int p) {
int i, gcd = 1;

    for(i=1; i <= q && i <= p; ++i)
    {
             if(q % i ==0 && p % i ==0)
                        gcd = i;
    }

return gcd;
}
jgaeddert commented 1 year ago

You are correct in that there is a logic error with how the GCD is computed. This has been fixed in d78f5c1aa91643d64863d97005db57e36458c4ab and with a few added tests.