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

Tighten the type restriction for getproperty convenience functions #34

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

Conversation

dingraha
Copy link
Collaborator

Sorry to raise this issue again @andrewning. I checked out the changes you mentioned in #33, and they do fix the problem I was seeing, but I'm getting errors with some fancy array packages that I use occasionally with CCBlade.jl. For example, on master right now, if I try to construct an OffsetArray of OperatingPoint structs:

julia> using CCBlade, OffsetArrays
Precompiling CCBlade...
  1 dependency successfully precompiled in 3 seconds. 48 already precompiled.

julia> radii = range(0.0, 1.0; length=10)
0.0:0.1111111111111111:1.0

julia> ops = OperatingPoint.(similar(radii), similar(radii), similar(radii), similar(radii), similar(radii), similar(radii))
10-element Vector{OperatingPoint{Float64}}:
 OperatingPoint{Float64}(6.92304934426196e-310, 6.9230543648701e-310, 6.9230563392829e-310, 6.9230474992928e-310, 5.0e-324, 6.92305733658365e-310)
 OperatingPoint{Float64}(6.92305607079577e-310, 6.9230543649381e-310, 6.92305607079577e-310, 6.9230474992928e-310, 5.0e-324, 6.92305733658365e-310)
 OperatingPoint{Float64}(6.9230563392829e-310, 6.9230543646203e-310, 6.9230563208483e-310, 6.9230474992928e-310, 5.0e-324, 6.92305733683306e-310)
 OperatingPoint{Float64}(6.92304746166553e-310, 6.92296044673307e-310, 6.9230500560975e-310, 6.9230474992928e-310, 0.0, 6.92305733683306e-310)
 OperatingPoint{Float64}(6.92305733687733e-310, 0.0, 0.0, 0.0, 0.0, 6.92305733658365e-310)
 OperatingPoint{Float64}(6.92305733687733e-310, 0.0, 0.0, 0.0, 0.0, 6.92305733658365e-310)
 OperatingPoint{Float64}(6.92305733689946e-310, 0.0, 0.0, 0.0, 0.0, 6.9230573368552e-310)
 OperatingPoint{Float64}(6.92296044668327e-310, 0.0, 0.0, 0.0, 0.0, 6.9230573368552e-310)
 OperatingPoint{Float64}(6.92305733694373e-310, 0.0, 0.0, 0.0, 0.0, 6.92305733687733e-310)
 OperatingPoint{Float64}(6.92305733696586e-310, 0.0, 0.0, 0.0, 0.0, 6.92305733689946e-310)

julia> ops_oa = OffsetArray(ops, 0:length(ops)-1)
Error showing value of type OffsetVector{OperatingPoint{Float64}, Vector{OperatingPoint{Float64}}}:
ERROR: StackOverflowError:
Stacktrace:
     [1] materialize(bc::Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(getfield), Tuple{OffsetVector{OperatingPoint{Float64}, Vector{OperatingPoint{Float64}}}, Base.RefValue{Sym
bol}}})
       @ Base.Broadcast ./broadcast.jl:867
     [2] getproperty(obj::OffsetVector{OperatingPoint{Float64}, Vector{OperatingPoint{Float64}}}, sym::Symbol)
       @ CCBlade ~/projects/aviary_propeller/dev/CCBlade/src/CCBlade.jl:135
     [3] parent(A::OffsetVector{OperatingPoint{Float64}, Vector{OperatingPoint{Float64}}})
       @ OffsetArrays ~/.julia/packages/OffsetArrays/HLmxQ/src/OffsetArrays.jl:281
     [4] axes(A::OffsetVector{OperatingPoint{Float64}, Vector{OperatingPoint{Float64}}})
       @ OffsetArrays ~/.julia/packages/OffsetArrays/HLmxQ/src/OffsetArrays.jl:295
     [5] combine_axes(A::OffsetVector{OperatingPoint{Float64}, Vector{OperatingPoint{Float64}}}, B::Base.RefValue{Symbol})
       @ Base.Broadcast ./broadcast.jl:492
     [6] instantiate(bc::Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(getfield), Tuple{OffsetVector{OperatingPoint{Float64}, Vector{OperatingPoint{Float64}}}, Base.RefValue{Sym
bol}}})
       @ Base.Broadcast ./broadcast.jl:302
--- the above 6 lines are repeated 4058 more times ---
 [24355] materialize(bc::Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(getfield), Tuple{OffsetVector{OperatingPoint{Float64}, Vector{OperatingPoint{Float64}}}, Base.RefValue{Sym
bol}}})
       @ Base.Broadcast ./broadcast.jl:867
 [24356] getproperty(obj::OffsetVector{OperatingPoint{Float64}, Vector{OperatingPoint{Float64}}}, sym::Symbol)
       @ CCBlade ~/projects/aviary_propeller/dev/CCBlade/src/CCBlade.jl:135
 [24357] parent(A::OffsetVector{OperatingPoint{Float64}, Vector{OperatingPoint{Float64}}})
       @ OffsetArrays ~/.julia/packages/OffsetArrays/HLmxQ/src/OffsetArrays.jl:281
 [24358] length
       @ ~/.julia/packages/OffsetArrays/HLmxQ/src/OffsetArrays.jl:293 [inlined]
 [24359] isempty(a::OffsetVector{OperatingPoint{Float64}, Vector{OperatingPoint{Float64}}})
       @ Base ./abstractarray.jl:1212
 [24360] show(io::IOContext{Base.TTY}, ::MIME{Symbol("text/plain")}, X::OffsetVector{OperatingPoint{Float64}, Vector{OperatingPoint{Float64}}})
       @ Base ./arrayshow.jl:364
 [24361] (::REPL.var"#68#69"{REPL.REPLDisplay{REPL.LineEditREPL}, MIME{Symbol("text/plain")}, Base.RefValue{Any}})(io::Any)
       @ REPL ~/local/julia/1.11.2/share/julia/stdlib/v1.11/REPL/src/REPL.jl:367
 [24362] with_repl_linfo(f::Any, repl::REPL.LineEditREPL)
       @ REPL ~/local/julia/1.11.2/share/julia/stdlib/v1.11/REPL/src/REPL.jl:661
 [24363] display(d::REPL.REPLDisplay, mime::MIME{Symbol("text/plain")}, x::Any)
       @ REPL ~/local/julia/1.11.2/share/julia/stdlib/v1.11/REPL/src/REPL.jl:353
 [24364] display
       @ ~/local/julia/1.11.2/share/julia/stdlib/v1.11/REPL/src/REPL.jl:372 [inlined]
 [24365] display(x::Any)
       @ Base.Multimedia ./multimedia.jl:340
 [24366] #invokelatest#2
       @ ./essentials.jl:1055 [inlined]
 [24367] invokelatest
       @ ./essentials.jl:1052 [inlined]
 [24368] print_response(errio::IO, response::Any, show_value::Bool, have_color::Bool, specialdisplay::Union{Nothing, AbstractDisplay})
       @ REPL ~/local/julia/1.11.2/share/julia/stdlib/v1.11/REPL/src/REPL.jl:409
 [24369] (::REPL.var"#70#71"{REPL.LineEditREPL, Pair{Any, Bool}, Bool, Bool})(io::Any)
       @ REPL ~/local/julia/1.11.2/share/julia/stdlib/v1.11/REPL/src/REPL.jl:378
 [24370] with_repl_linfo(f::Any, repl::REPL.LineEditREPL)
       @ REPL ~/local/julia/1.11.2/share/julia/stdlib/v1.11/REPL/src/REPL.jl:661
 [24371] print_response(repl::REPL.AbstractREPL, response::Any, show_value::Bool, have_color::Bool)
       @ REPL ~/local/julia/1.11.2/share/julia/stdlib/v1.11/REPL/src/REPL.jl:376
 [24372] (::REPL.var"#do_respond#96"{Bool, Bool, REPL.var"#112#130"{REPL.LineEditREPL, REPL.REPLHistoryProvider}, REPL.LineEditREPL, REPL.LineEdit.Prompt})(s::REPL.LineEdit.MIState, buf::Any, ok::Bool)
       @ REPL ~/local/julia/1.11.2/share/julia/stdlib/v1.11/REPL/src/REPL.jl:1003
 [24373] #invokelatest#2
       @ ./essentials.jl:1055 [inlined]
 [24374] invokelatest
       @ ./essentials.jl:1052 [inlined]
 [24375] run_interface(terminal::REPL.Terminals.TextTerminal, m::REPL.LineEdit.ModalInterface, s::REPL.LineEdit.MIState)
       @ REPL.LineEdit ~/local/julia/1.11.2/share/julia/stdlib/v1.11/REPL/src/LineEdit.jl:2755
 [24376] run_frontend(repl::REPL.LineEditREPL, backend::REPL.REPLBackendRef)
       @ REPL ~/local/julia/1.11.2/share/julia/stdlib/v1.11/REPL/src/REPL.jl:1474

julia> 

I get similar errors if I try to use a Fill array from the FillArrays.jl package. The fix I found was to restrict the getproperty convenience functions to Vector instead of AbstractVector. Also the code now checks for both the ref and size symbols, since those are AFIACT the only two fields that exist in the plain Array Julia type.

I added tests for creating OffsetArrays and FillArrays, FYI.

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.

1 participant