-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
Add abstract complex #35587
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
Add abstract complex #35587
Conversation
|
Something perhaps worth discussing: should we have This slight possible advantage is likely not worth it though. Interested to hear if anyone has thoughts or feelings on the matter. |
|
From a programming perspective rather than a mathematical perspective, why is
|
|
As real numbers are a strict subset of complex numbers, I’d be in favor of |
|
Regarding the type parameter: yes, it seems like We're not doing |
The thing that worried me about the type parameter in struct HeterogeneousComplex{T, U}
re::T
im::U
endbut I guess this is a rather small corner case and whoever implemented such a thing can just override the Base methods still. I'll add in the type parameter. |
|
@Roger-luo are you still interested in collaborating on this? If you or anyone else is interested, I could use some help coming up with test cases and docs. I'm also not sure what to do with promotion rules so (if anything) suggestions on that front would be appreciated. |
abstract type AbstractComplex{Tre<:Real, Tim<:Real} <: Number end
struct Complex{T} <: AbstractComplex{T, T}
re::T
im::T
end
struct HeterogeneousComplex{T, U} <: AbstractComplex{T, U}
re::T
im::U
end😄 |
|
Yes, that is a possibility, but would such a thing be worth the complication it adds? I suspect no but I'm open to arguments. |
|
I cannot imagine other practical complex types except the polar representation. So, I think one parameter is sufficient. (Since trailing parameters can be omitted, I also think the 2-parameter type is not so "complex".) |
|
I don't really understand what properties the As an end user, I welcome this change, but as a package developer, I'm reluctant to support |
The difference is that if you are using |
|
Yes. So I'm trying to understand "the right interface". If it satisfies the right interface, my code will work with the I think I'm not against types like |
Split complex numbers are a bit of a harrier situation. I think you're be right to be suspicious of them being |
This comment has been minimized.
This comment has been minimized.
|
Hi, sorry for this late reply. My originally motivation for A more concrete use case is the symbolic complex type, that treats a symbol as a complex number instead of two variable that is subtype of struct SComplex <: AbstractComplex
ex
end
x = SComplex(:z) the return value of Expr
head: Symbol call
args: Array{Any}((2,))
1: Symbol real
2: SComplexthus you can't really know what makes sense to put into |
|
What I'm proposing here is struct Complex{T} <: AbstractComplex
re::T
im::T
endand for other types struct SComplex <: AbstractComplex
ex
end
struct Im{T} <: AbstractComplex
im::T
end
struct Polar{T} <: AbstractComplex
re::T
im::T
endI don't think we really need a type parameter, even if we need one, the better way is to use |
|
If |
|
yet another (semi-symbolic) representation of complex numbers: In https://github.com/kalmarek/Cyclotomics.jl I use julia> x = E(5)^2 + E(5)^3
ζ₅² +ζ₅³
julia> dump(x)
Cyclotomics.Cyclotomic{Int64,SparseArrays.SparseVector{Int64,Int64}}
n: Int64 5
coeffs: SparseArrays.SparseVector{Int64,Int64}
n: Int64 5
nzind: Array{Int64}((2,)) [3, 4]
nzval: Array{Int64}((2,)) [1, 1]
i.e. represent sums of roots of unity as elements of the appropriate vector space over julia> E(45)^9
ζ₅
julia> E(45)^5
-ζ₉⁴ -ζ₉⁷
julia> E(45)^5 + 2E(45)^9
ζ₄₅² +ζ₄₅⁸ +ζ₄₅¹¹ +ζ₄₅¹⁷ -2*ζ₄₅²⁴ +ζ₄₅²⁶ +ζ₄₅²⁹ +ζ₄₅³⁸ -2*ζ₄₅³⁹ +ζ₄₅⁴⁴
clearly this is not julia> typeof(1.0*E(5,3))
Cyclotomics.Cyclotomic{Float64,SparseArrays.SparseVector{Float64,Int64}}
I'd like to make it into |
|
Bump :) I ran into an application where having a well supported Polar representation type would be useful :). What remains to be decided here? |
|
I think showing any usefulness in practice at having the abstract type, cf #33246 (comment) |
@Keno I think some people (like me) don't think there should be a type parameter for the abstract type since it can be just a constant or some struct with an empty field like The rest I think looks good to me. |
|
I don't have any strong feelings one way or another on whether or not it should be parameterized. I can see benefits and downsides to either. Keno, since you want this, do you have any intuitions regarding parameterizing |
I think there should be a type parameter: the type parameter tells you what kind of value to expect when you ask for the real or imaginary parts of the number. That does not force the implementation to store a value of that type or any value for that matter. If the value is a constant you still have to return a value when asking for the real/imaginary parts and whatever that type is is what the type parameter should be. |
|
Right, you can always set the type parameter to I don't understand the |
|
I think the idea is that in your symbolic system, you'd write something like z = SComplex(:z)
x = SReal(:x)
ex1= simplify(z^2 >= 0)
ex2 = simplify(x^2 >= 0and have It's directly anaologous to wanting a symbolic representation of an array, rather than an array of symbolic scalars. |
|
From triage:
|
|
After some thinking about the parameteric solution, I think actually since there is always a symbolic term type in CAS system, e.g struct SComplex <: AbstractComplex{Term}
ex::Term
endand |
|
So what should the minimal interface be? Just There's some code here that feels like it could be made more generic, like Lines 290 to 292 in f1915bc
But I'm not sure what the generalization would be. Would the fallbacks just convert to Perhaps we could make part of the interface that your complex type needs a way to take in real and imaginary parts to make an instance, e.g. every Then for instance, function muladd(z::AbstractComplex, w::AbstractComplex, x::AbstractComplex)
T = promote_type(typeof(z), typeof(w), typeof(x))
T(real = muladd(real(z), real(w), -muladd(imag(z), imag(w), -real(x))),
imag = muladd(real(z), imag(w), muladd(imag(z), real(w), imag(x))))
endUsers could then make their own more efficient methods if they wanted. |
I think so, this is consistent with other things in Base too. |
|
Requiring people to make such a constructor with kwargs seemed like a nonstandard thing to do, so instead I'm going to try something like for unary in (:conj, :inv, :+, :-, :sqrt, :cis, :log, :cispi, :angle, :log10, :log2, :exp, :expm1, :log1p, :exp2, :exp10,
:sin, :cos, :tan, :asin, :acos, :atan, :sinh, :cosh, :tanh, :asinh, :acosh, :atanh, :float, :big)
@eval $unary(z::AbstractComplex) = (typeof(z) ∘ $unary ∘ Complex)(z)
end
for binary in (:+, :-, :*, :/, :^, :muladd)
@eval $binary(z::AbstractComplex, w::AbstractComplex) = (promote_typeof(z, w) ∘ $binary)(Complex(z), Complex(w))
@eval $binary(z::Real, w::AbstractComplex) = (promote_typeof(z, w) ∘ $binary)(z, Complex(w))
@eval $binary(z::AbstractComplex, w::Real) = (promote_typeof(z, w) ∘ $binary)(Complex(z), w)
endThis way, to get a generic implementation, you just need to be able to convert back and forth between your type and |
|
bump. What is needed to get this merged? |
|
While we're at it, is it an idea to implement an |
|
what behavior would it have? |
|
It's only tangential to this PR, sorry, but I'll explain where I'm coming from. I was trying to define the sets of natural numbers, of integers, real numbers and rational numbers in But there are no For a number, just like Still might not be used much :-) |
|
Closes #33246
I said I'd do this a while ago and next got around to it. I'm sure there's a lot more machinery that can be put into this, but I figured this was approximately the minimal set of methods that
AbstractComplexshould satisfy, with the rest being up to whoever implements other complex types.