Hi Luis, I pushed some changes to the branch 'complex_atan2'. Please have a look, add more tests if you're able, and above all make sure I didn't break anything.
cheers, Derek > On Dec 6, 2018, at 11:35 PM, Derek Lamb <[email protected]> wrote: > > 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 > _______________________________________________ pdl-general mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/pdl-general
