Thanks Derek. I'll look asap.
Best regards,
Luis
On Fri, Dec 07, 2018 at 04:14:43PM -0700, Derek Lamb wrote:
> 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
> >
>
>
--
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