Hi Luis, Thanks for the bug report.
As you point out, these functions are undocumented. At first that suggests to me that they are for internal use only. But then I see that they overload the Perl operator atan2. So I guess it needs to be corrected! Using your implementation with the complex log, I get the expected result: perl -Mblib -MPDL -MPDL::Complex -wE 'say atan2(pdl(1)->r2C,pdl(0)->r2C);' 1.5708 +0i (although atan2 should not return a complex number.) Also note that there is already Carg, which does the same thing: pdl> p Carg(0+1*i); 1.5707963267949 PDL::Complex is seriously lacking in tests. Do you have any interest in contributing some tests to t/complex.t? To illustrate the problem, 'make test' succeeds with the current git, when I swap the arguments in the subs Catan2 and atan2, and when I replace them with the complex log as you suggested. That means the Catan2 code is never exercised by the test suite. Almost certainly there are other functions that don't get exercised either. Evil edge cases are best, like the one you found here, but even just making sure each subroutine gets exercised once in the test suite helps, too. I also noticed that the documentation is lacking: a list of overloaded operators is clearly needed, but that shouldn't be too hard. best, Derek > On Dec 5, 2018, at 11:26 AM, Luis Mochan <[email protected]> wrote: > > I believe I found a mistake in the undocumented PDL::Complex functions Catan2 > and atan2. > > pdl> use PDL::Complex > pdl> p atan2(1,0) # perl's function yields PI/2 > 1.5707963267949 > pdl> p atan2(pdl(1),pdl(0)) # PDL's function also > 1.5707963267949 > pdl> p atan2(pdl(1)->r2C,pdl(0)->r2C) # but PDL::Complex's function disagrees > 0 +0i > pdl> p atan2(pdl(0)->r2C, pdl(1)->r2C) # The first argument can't be zero. > NaN +NaNi > pdl> p atan2(pdl(0.000001)->r2C, pdl(1)->r2C) # But if ~0 result is not the > expected 0 > 1.5708 -5.55112e-17i > pdl> > > I believe the error is in lines 905 and 906 of Basic/Complex/complex.pd > > sub Catan2($$) { Catan Cdiv $_[1], $_[0] } > sub atan2($$) { Catan Cdiv $_[1], $_[0] } > > where the 1's and 0's should be swapped. > > sub Catan2($$) { Catan Cdiv $_[0], $_[1] } > sub atan2($$) { Catan Cdiv $_[0], $_[1] } > > I guess it would be even better to use the complex log, > > sub Catan2($$) { Clog( $_[1] + i*$_[0])/i } > > as it wouldn't fail if the second argument is zero; it's real part > would coincide with the usual real atan2 of two real arguments. > > > Best regards, > Luis > > -- > > o > W. Luis Mochán, | tel:(52)(777)329-1734 /<(*) > Instituto de Ciencias Físicas, UNAM | fax:(52)(777)317-5388 `>/ /\ > Apdo. Postal 48-3, 62251 | (*)/\/ \ > Cuernavaca, Morelos, México | [email protected] /\_/\__/ > GPG: 791EB9EB, C949 3F81 6D9B 1191 9A16 C2DF 5F0A C52B 791E B9EB > > > > > _______________________________________________ > pdl-general mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/pdl-general > _______________________________________________ pdl-general mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/pdl-general
