Skip to content

Conversation

@YingboMa
Copy link
Member

@YingboMa YingboMa commented Aug 7, 2020

julia> foo(u0) = sqrt(-u0[1])
foo (generic function with 1 method)

julia> foo(u0)
ERROR: DomainError with -2.8e6:
sqrt will only return a complex result if called with a complex argument. Try sqrt(Complex(x)).
Stacktrace:
 [1] throw_complex_domainerror(::Symbol, ::Float64) at ./math.jl:33
 [2] sqrt at ./math.jl:573 [inlined]
 [3] foo(::ComponentArray{Float64,1,Array{Float64,1},Tuple{Axis{(outdoor_hex = ViewAxis(1:20, Axis((Ps = 1:4, hs = 5:8, Ms = 9:12, Us = 13:16, TWalls = 17:20))), indoor_hex = ViewAxis(21:40, Axis((Ps = 1:4, hs = 5:8, Ms = 9:12, Us = 13:16, TWalls = 17:20))))}}}) at ./REPL[481]:1
 [4] top-level scope at REPL[482]:1

julia> Base.show(io::IO, ::Type{<:ComponentArray}) = print(io, "ComponentArray")

julia> foo(u0)
ERROR: DomainError with -2.8e6:
sqrt will only return a complex result if called with a complex argument. Try sqrt(Complex(x)).
Stacktrace:
 [1] throw_complex_domainerror(::Symbol, ::Float64) at ./math.jl:33
 [2] sqrt at ./math.jl:573 [inlined]
 [3] foo(::ComponentArray) at ./REPL[481]:1
 [4] top-level scope at REPL[484]:1

@codecov-commenter
Copy link

codecov-commenter commented Aug 7, 2020

Codecov Report

Merging #42 into master will decrease coverage by 2.64%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #42      +/-   ##
==========================================
- Coverage   58.06%   55.41%   -2.65%     
==========================================
  Files          16       16              
  Lines         403      406       +3     
==========================================
- Hits          234      225       -9     
- Misses        169      181      +12     
Impacted Files Coverage Δ
src/show.jl 0.00% <0.00%> (ø)
src/componentindex.jl 50.00% <0.00%> (-50.00%) ⬇️
src/set_get.jl 75.86% <0.00%> (-6.90%) ⬇️
src/broadcasting.jl 50.90% <0.00%> (-3.64%) ⬇️
src/similar_convert_copy.jl 74.28% <0.00%> (-2.86%) ⬇️
src/componentarray.jl 77.89% <0.00%> (-1.06%) ⬇️
src/axis.jl 71.87% <0.00%> (-0.86%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9288f43...e94d14f. Read the comment docs.

@jonniedie
Copy link
Collaborator

jonniedie commented Aug 7, 2020

I like this, but I think I want to add an additional method like Base.show(io::IO, ::MIME"text/plain", ::Type{ComponentArray{T,N,A,Ax}}) where {T,N,A,Ax} = print(io, "ComponentArray{$T,$N,$A,$Ax}") so you can still inspect the type directly via typeof(u0[1]).

Although I wonder if the it would be better to have the eltype printed as well. Because it's reasonable that someone might have a method defined on a ComponentArray with a certain eltype and it would be frustrating for this situation to happen:

julia> sum_magnitudes(x::ComponentArray{<:Complex}) = sum(abs, x)
sum_magnitudes (generic function with 1 method)

julia> ca = ComponentArray(a=42, b=rand(3))
ComponentVector{Float64}(a = 42.0, b = [0.7541071306370861, 0.16224993425651957, 0.30748376563379165])

julia> sum_magnitudes(ca)
ERROR: MethodError: no method matching sum_magnitudes(::ComponentArray)
Closest candidates are:
  sum_magnitudes(::ComponentArray) at REPL[9]:1
Stacktrace:
 [1] top-level scope at REPL[13]:1
 [2] include_string(::Function, ::Module, ::String, ::String) at .\loading.jl:1088

I'm going to give this some thought. ComponentVectors and ComponentMatrices are already printing similarly, this would just make it the same for their types too:

julia> ca
ComponentVector{Float64}(a = 42.0, b = [0.7541071306370861, 0.16224993425651957, 0.30748376563379165])

julia> ca * ca'
ComponentMatrix{Float64} with axes (a = 1, b = 2:4) × (a = 1, b = 2:4)
 1764.0     31.6725    6.8145     12.9143
   31.6725   0.568678  0.122354    0.231876
    6.8145   0.122354  0.026325    0.0498892
   12.9143   0.231876  0.0498892   0.0945463

Or maybe including the dimension size would be good too, so it would mirror normal arrays?

@YingboMa
Copy link
Member Author

YingboMa commented Aug 8, 2020

Now it looks like this

julia> foo(u0) = sqrt(-u0[1])
foo (generic function with 1 method)

julia> foo(u0)
ERROR: DomainError with -2.8e6:
sqrt will only return a complex result if called with a complex argument. Try sqrt(Complex(x)).
Stacktrace:
 [1] throw_complex_domainerror(::Symbol, ::Float64) at ./math.jl:33
 [2] sqrt at ./math.jl:573 [inlined]
 [3] foo(::ComponentVector{Float64}) at ./REPL[4]:1
 [4] top-level scope at REPL[5]:1

julia> foo(u0*u0')
ERROR: DomainError with -7.84e12:
sqrt will only return a complex result if called with a complex argument. Try sqrt(Complex(x)).
Stacktrace:
 [1] throw_complex_domainerror(::Symbol, ::Float64) at ./math.jl:33
 [2] sqrt at ./math.jl:573 [inlined]
 [3] foo(::ComponentMatrix{Float64}) at ./REPL[4]:1
 [4] top-level scope at REPL[6]:1

julia> foo(u0.*u0'.*reshape(u0, (1,1,length(u0))))
ERROR: DomainError with -2.1952e19:
sqrt will only return a complex result if called with a complex argument. Try sqrt(Complex(x)).
Stacktrace:
 [1] throw_complex_domainerror(::Symbol, ::Float64) at ./math.jl:33
 [2] sqrt at ./math.jl:573 [inlined]
 [3] foo(::ComponentArray{Float64, 3}) at ./REPL[4]:1
 [4] top-level scope at REPL[7]:1

@jonniedie
Copy link
Collaborator

That's, perfect. Thanks!

@jonniedie jonniedie merged commit 9e521ea into SciML:master Aug 8, 2020
@YingboMa YingboMa deleted the myb/show branch August 16, 2020 03:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants