-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
No more ImaginaryUnit: const im = Complex(false,true)
.
#5468
Conversation
This change requires giving multiplition by Bools special behavior. Approximately, `true * x = x` and `false * x = zero(x)`, but a bit complicated for the sake of promotion, the only non-trivial example of which is `Bool * MathConst`, which is promoted to Float64. Idea originally due to @GunnarFarneback: https://groups.google.com/forum/#!topic/julia-dev/VkGrqnrAdaY #2980 #3728
This change is hard to resist. It simplifies the code and adds nice behavior like In the issues linked above, I said I couldn't think of a justification for this. Now I might have one: a pure imaginary type behaves identically to a complex number whose real part behaves like the |
@@ -42,6 +42,12 @@ abs2(x::Bool) = x | |||
^(x::Bool, y::Bool) = x|!y | |||
^(x::Integer, y::Bool) = y ? x : one(x) | |||
|
|||
function *{T<:Number}(x::Bool, y::T) | |||
S = promote_type(Bool,T) | |||
ifelse(x, convert(S,y), convert(S,0)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just curious, why the use of ifelse
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't require a branch --- both clauses can be evaluated, which often allows better code generation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
I like it; I don't see any use for |
Alright. If it's good enough for @stevengj, there's not much more I could ask for. The only way we're going to find out if this is ok is by merging it. |
No more ImaginaryUnit: `const im = Complex(false,true)`.
is a lot longer than |
Working on improving the codegen now. |
Codegen is much improved, but it still does an unnecessary addition of |
We were hoping to avoid a pure imaginary type. But, in addition to the codegen issue, there is also a potential bug:
|
What is |
Steven's function |
Ok, the core problem is that |
This change requires giving multiplition by Bools special behavior. Approximately,
true * x = x
andfalse * x = zero(x)
, but a bit complicated for the sake of promotion, the only non-trivial example of which isBool * MathConst
, which is promoted to Float64.Idea originally due to @GunnarFarneback:
Compare against #5313.