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

Define complex sqrt #374

Merged
merged 1 commit into from
Jun 20, 2024
Merged

Define complex sqrt #374

merged 1 commit into from
Jun 20, 2024

Conversation

mtfishman
Copy link
Contributor

@mtfishman mtfishman commented Jun 19, 2024

Fixes #364.

@maleadt this was the most minimal implementation I could come up with.

I override two functions:

  1. Base.Math.exponent(::Base.IEEEFloat) in order to avoid exception throws.
  2. Base.ssqs(x::T, y::T) where T<:Real to avoid a Union type in type inference by changing this line to k = m==0 ? 0 : exponent(m).

I wonder if 2. should be fixed in Base. The Base version effectively sets k to zero(m) rather than 0 if iszero(m), however k is supposed to be Int anyway. The way that I changed it, it would probably mean that Base wouldn't need to have so many type asserts for k (there is one in Base.ssqs and one in Base.sqrt(z::Complex)).

@maleadt
Copy link
Member

maleadt commented Jun 20, 2024

Thanks! It's too bad we have to do this; I should probably look into implementing a dynamic memory allocator.

I wonder if 2. should be fixed in Base.

I guess so, even though the exception branches don't generally hurt CPU performance it's easier on the compiler to avoid them where possible.

@maleadt maleadt merged commit 59c7f1a into JuliaGPU:main Jun 20, 2024
1 check passed
@maleadt maleadt added bug kernels Things about kernels and how they are compiled. labels Jun 20, 2024
@mtfishman mtfishman deleted the complex_sqrt branch June 20, 2024 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kernels Things about kernels and how they are compiled.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sqrt(::Complex) unsupported due to conversion exceptions
2 participants