Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

complex ldexp #32336

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

hamzaelsaawy
Copy link

Defining ldexp for complex type as a map over individual elements

hamzaelsaawy and others added 2 commits June 15, 2019 16:18
Rather not define for a subset of parameters for `Complex` to allow for user defined types `<:Number`
Modified ldexp tests from base/math
@ViralBShah ViralBShah added the domain:maths Mathematical functions label Jun 17, 2019
Copy link
Member

@c42f c42f left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neither C++ nor numpy seem to support complex ldexp - can you find out why that is?

@@ -172,6 +176,55 @@ end
end
end

@testset "complex ldexp" begin
@testset "Complex{$T}" for F in (Float16,Float32,Float64)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure it's worth just copying the tests from the the float versions of ldexp wholesale, given that the implementations are tightly coupled. It might be better just to test a few examples, given that complex ldexp inherits its behavior directly from the float version.

@hamzaelsaawy
Copy link
Author

I think ldexp in math.h originally wrapped an assembly instruction and there was no similar instruction for complex numbers. (Though interestingly the C++ implementation has a generic for integers).

And I will update the tests hopefully soon.

@c42f
Copy link
Member

c42f commented Jul 22, 2019

Sure, I'm just wondering if numpy did this on purpose; they do state very explicitly/authoritatively in their documentation that "Complex dtypes are not supported, they will raise a TypeError.".

Ok, so after digging into the numpy history, it appears there's no good reason for this. Travis Oliphant added ldexp for normal floats way back in numpy/numpy@d91b5b9. The documentation eventually caught up in numpy/numpy@87fa5ae. The answer could very well be "it was annoying to add to the C code so nobody bothered" :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:maths Mathematical functions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants