Closed zygurt closed 8 years ago
Any idea on the most elegant way to fix this?
zygurt notifications@github.com writes:
Complex_phase and pol_to_cart in math.xtm are not quadrant friendly, and should be.
You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/digego/extempore/issues/262
atan2 is quadrant friendly. (atan (/ (tref a 1) (tref a 0))) becomes (atan2 (tref a 1) (tref a 0))
Ok, great. I'm happy to make the change myself, but if you want to submit a pull request and win the associated internet points, I'm happy to do it that way as well :)
zygurt notifications@github.com writes:
atan2 is quadrant friendly. (atan (/ (tref a 1) (tref a 0))) becomes (atan2 (tref a 1) (tref a 0))
You are receiving this because you commented. Reply to this email directly or view it on GitHub: https://github.com/digego/extempore/issues/262#issuecomment-215602489
Let me just double check that it does what it's meant to do. I'll let you do it, as I don't know the process. I'll learn the process later, just not now. This is Tim from the google group btw.
Ok, no worries. Thanks Tim.
zygurt notifications@github.com writes:
Let me just double check that it does what it's meant to do. I'll let you do it, as I don't know the process. I'll learn the process later, just not now. This is Tim from the google group btw.
You are receiving this because you commented. Reply to this email directly or view it on GitHub: https://github.com/digego/extempore/issues/262#issuecomment-215603770
Turns out that the pol_to_cart and cart_to_pol closures needed some work. Below are the working versions that fix the following issues. 1) While transforming in place, the new real or magnitude value would be used when calculating the new imaginary or theta value. Added a temporary variable to store element 0 of Complexf*. 2) Closure wasn't moving through buffer when selecting values to use. It would use the base memory location for all calculations. Changed a to (pref a i) where needed. 3) cart_to_pol is now quadrant friendly and uses atan2.
(bind-func Complex_phase (lambda (a:Complexf) (atan2 (tref a 1) (tref a 0))))
(bind-func cart_to_pol (lambda (a:Complexf) (Cpxf (sqrt (+ (* (tref a 0) (tref a 0)) (* (tref a 1) (tref a 1)))) (atan2 (tref a 1) (tref a 0)))))
(bind-func cart_to_pol "transform a whole buffer cart->pol in-place" (lambda (a:Complexf* n:i64) (let ((temp:float 0.0)) (doloop (i n) (set! temp (sqrt (+ (* (tref (pref a i) 0) (tref (pref a i) 0)) (* (tref (pref a i) 1) (tref (pref a i) 1))))) (tset! (pref-ptr a i) 1 (atan2 (tref (pref a i) 1) (tref (pref a i) 0))) (tset! (pref-ptr a i) 0 temp)) void)))
(bind-func pol_to_cart "transform a whole buffer pol->cart in-place" (lambda (a:Complexf* n:i64) (let ((temp:float 0.0)) (doloop (i n) (set! temp (* (tref (pref a i) 0) (cos (tref (pref a i) 1)))) (tset! (pref-ptr a i) 1 (* (tref (pref a i) 0) (sin (tref (pref a i) 1)))) (tset! (pref-ptr a i) 0 temp)) void)))
The same changes will need to be made to the double versions of the closures. These were tested with: (fft temp_buff spectrum fft_size) (cart_to_pol2 spectrum spectrum_size) (pol_to_cart2 spectrum spectrum_size) (ifft spectrum temp_buff2 fft_size)
EDIT: Aww, the formatting disappears when clicking comment.
Great, thanks Tim. Will update it when I get a chance.
zygurt notifications@github.com writes:
Turns out that the pol_to_cart and cart_to_pol closures needed some work. Below are the working versions that fix the following issues. 1) While transforming in place, the new real or magnitude value would be used when calculating the new imaginary or theta value. Added a temporary variable to store element 0 of Complexf*. 2) Closure wasn't moving through buffer when selecting values to use. It would use the base memory location for all calculations. Changed a to (pref a i) where needed. 3) cart_to_pol is now quadrant friendly and uses atan2.
(bind-func Complex_phase (lambda (a:Complexf) (atan2 (tref a 1) (tref a 0))))
(bind-func cart_to_pol (lambda (a:Complexf) (Cpxf (sqrt (+ (* (tref a 0) (tref a 0)) (* (tref a 1) (tref a 1)))) (atan2 (tref a 1) (tref a 0)))))
(bind-func cart_to_pol "transform a whole buffer cart->pol in-place" (lambda (a:Complexf* n:i64) (let ((temp:float 0.0)) (doloop (i n) (set! temp (sqrt (+ (* (tref (pref a i) 0) (tref (pref a i) 0)) (* (tref (pref a i) 1) (tref (pref a i) 1))))) (tset! (pref-ptr a i) 1 (atan2 (tref (pref a i) 1) (tref (pref a i) 0))) (tset! (pref-ptr a i) 0 temp)) void)))
(bind-func pol_to_cart "transform a whole buffer pol->cart in-place" (lambda (a:Complexf* n:i64) (let ((temp:float 0.0)) (doloop (i n) (set! temp (* (tref (pref a i) 0) (cos (tref (pref a i) 1)))) (tset! (pref-ptr a i) 1 (* (tref (pref a i) 0) (sin (tref (pref a i) 1)))) (tset! (pref-ptr a i) 0 temp)) void)))
The same changes will need to be made to the double versions of the closures. These were tested with: (fft temp_buff spectrum fft_size) (cart_to_pol2 spectrum spectrum_size) (pol_to_cart2 spectrum spectrum_size) (ifft spectrum temp_buff2 fft_size)
You are receiving this because you commented. Reply to this email directly or view it on GitHub: https://github.com/digego/extempore/issues/262#issuecomment-215609511
Complex_phase and pol_to_cart in math.xtm are not quadrant friendly, and should be.