v2; make StateSpaceSet subtype Vector{<:AbstractVector}

[Datseris]

Closes #18

Now downstream packages can decide the type of the stored vectors by passing in the container keyword to any of StateSpaceSet constructors. Also now that we subtype AbstractVector directly, the whole Julia ecosystem will work out of the box (hopefully).

[kahaaga]

@Datseris Is the intention here to allow the element type to be any input vector (i.e. AbstractVector{T} where T <: Any), or are there restrictions on T?

Constructing a StateSpaceSet with any element type works, but showing it doesn't (see example below). This is because mat = zeros(eltype(d), 50, D) obviously won't work unless eltype(d) is numeric.

MWE

julia> StateSpaceSet([rand(some_collection, 3) for i = 1:100]);

julia> some_collection = ["haha", (1, 3, 6), 2.2, 5];

julia> StateSpaceSet([rand(some_collection, 3) for i = 1:100]);

julia> StateSpaceSet([rand(some_collection, 3) for i = 1:100])
Error showing value of type StateSpaceSet{3, Any, SVector{3, Any}}:
ERROR: MethodError: no method matching zero(::Type{Any})

Closest candidates are:
  zero(::Type{Union{Missing, T}}) where T
   @ Base missing.jl:105
  zero(::Type{Union{}}, Any...)
   @ Base number.jl:310
  zero(::Type{Missing})
   @ Base missing.jl:104
  ...

Stacktrace:
  [1] zero(::Type{Any})
    @ Base ./missing.jl:106
  [2] zeros(::Type{Any}, dims::Tuple{Int64, Int64})
    @ Base ./array.jl:637
  [3] zeros(::Type{Any}, ::Int64, ::Int64)
    @ Base ./array.jl:632
  [4] matstring(d::StateSpaceSet{3, Any, SVector{3, Any}})
    @ StateSpaceSets ~/Documents/repos/sss/StateSpaceSets.jl/src/statespaceset.jl:138
  [5] show(io::IOContext{Base.TTY}, ::MIME{Symbol("text/plain")}, d::StateSpaceSet{3, Any, SVector{3, Any}})
    @ StateSpaceSets ~/Documents/repos/sss/StateSpaceSets.jl/src/statespaceset.jl:151
  [6] (::REPL.var"#55#56"{REPL.REPLDisplay{REPL.LineEditREPL}, MIME{Symbol("text/plain")}, Base.RefValue{Any}})(io::Any)
    @ REPL ~/.julia/juliaup/julia-1.10.4+0.aarch64.apple.darwin14/share/julia/stdlib/v1.10/REPL/src/REPL.jl:273
  [7] with_repl_linfo(f::Any, repl::REPL.LineEditREPL)
    @ REPL ~/.julia/juliaup/julia-1.10.4+0.aarch64.apple.darwin14/share/julia/stdlib/v1.10/REPL/src/REPL.jl:569
  [8] display(d::REPL.REPLDisplay, mime::MIME{Symbol("text/plain")}, x::Any)
    @ REPL ~/.julia/juliaup/julia-1.10.4+0.aarch64.apple.darwin14/share/julia/stdlib/v1.10/REPL/src/REPL.jl:259
  [9] display
    @ ~/.julia/juliaup/julia-1.10.4+0.aarch64.apple.darwin14/share/julia/stdlib/v1.10/REPL/src/REPL.jl:278 [inlined]
 [10] display(x::Any)
    @ Base.Multimedia ./multimedia.jl:340
 [11] print_response(errio::IO, response::Any, show_value::Bool, have_color::Bool, specialdisplay::Union{Nothing, AbstractDisplay})
    @ REPL ~/.julia/juliaup/julia-1.10.4+0.aarch64.apple.darwin14/share/julia/stdlib/v1.10/REPL/src/REPL.jl:0
 [12] (::REPL.var"#57#58"{REPL.LineEditREPL, Pair{Any, Bool}, Bool, Bool})(io::Any)
    @ REPL ~/.julia/juliaup/julia-1.10.4+0.aarch64.apple.darwin14/share/julia/stdlib/v1.10/REPL/src/REPL.jl:284
 [13] with_repl_linfo(f::Any, repl::REPL.LineEditREPL)
    @ REPL ~/.julia/juliaup/julia-1.10.4+0.aarch64.apple.darwin14/share/julia/stdlib/v1.10/REPL/src/REPL.jl:569
 [14] print_response(repl::REPL.AbstractREPL, response::Any, show_value::Bool, have_color::Bool)
    @ REPL ~/.julia/juliaup/julia-1.10.4+0.aarch64.apple.darwin14/share/julia/stdlib/v1.10/REPL/src/REPL.jl:282
 [15] (::REPL.var"#do_respond#80"{Bool, Bool, REPL.var"#93#103"{…}, REPL.LineEditREPL, REPL.LineEdit.Prompt})(s::REPL.LineEdit.MIState, buf::Any, ok::Bool)
    @ REPL ~/.julia/juliaup/julia-1.10.4+0.aarch64.apple.darwin14/share/julia/stdlib/v1.10/REPL/src/REPL.jl:911
 [16] #invokelatest#2
    @ ./essentials.jl:892 [inlined]
 [17] invokelatest
    @ ./essentials.jl:889 [inlined]
 [18] run_interface(terminal::REPL.Terminals.TextTerminal, m::REPL.LineEdit.ModalInterface, s::REPL.LineEdit.MIState)
    @ REPL.LineEdit ~/.julia/juliaup/julia-1.10.4+0.aarch64.apple.darwin14/share/julia/stdlib/v1.10/REPL/src/LineEdit.jl:2656
 [19] run_frontend(repl::REPL.LineEditREPL, backend::REPL.REPLBackendRef)
    @ REPL ~/.julia/juliaup/julia-1.10.4+0.aarch64.apple.darwin14/share/julia/stdlib/v1.10/REPL/src/REPL.jl:1312
 [20] (::REPL.var"#62#68"{REPL.LineEditREPL, REPL.REPLBackendRef})()
    @ REPL ~/.julia/juliaup/julia-1.10.4+0.aarch64.apple.darwin14/share/julia/stdlib/v1.10/REPL/src/REPL.jl:386
Some type information was truncated. Use `show(err)` to see complete types.

Solution

I see two solutions:

• do some input checks to only allow numeric eltypes
• modify the printing with a separate printing method for number types and other types, i.e.

function matstring(d::AbstractStateSpaceSet{D, T}) where {D, T<:Number}
    N = length(d)
    if N > 50
        mat = zeros(eltype(d), 50, D)
        for (i, a) in enumerate(flatten((1:25, N-24:N)))
            mat[i, :] .= d[a]
        end
    else
        mat = Matrix(d)
    end
    s = sprint(io -> show(IOContext(io, :limit=>true), MIME"text/plain"(), mat))
    s = join(split(s, '\n')[2:end], '\n')
    tos = summary(d)*"\n"*s
    return tos
end
function matstring(d::DS) where DS <: AbstractStateSpaceSet{D, T} where {D, T}
    V = typeof(d).name.name
    N = length(d)
    # ... plus some nicer matrix like printing when T is not number type
    return "$D-dimensional $V{$T} with $N points"
end

function matstring(d::AbstractStateSpaceSet{D, T}) where {D, T}
    N = length(d)
    if N > 50
        mat = zeros(eltype(d), 50, D)
        for (i, a) in enumerate(flatten((1:25, N-24:N)))
            mat[i, :] .= d[a]
        end
    else
        mat = Matrix(d)
    end
    s = sprint(io -> show(IOContext(io, :limit=>true), MIME"text/plain"(), mat))
    s = join(split(s, '\n')[2:end], '\n')
    tos = summary(d)*"\n"*s
    return tos
end

IMO, if we want the whole Julia ecosystem will work out of the box, we should be as permissive as possible and notify users that state space sets are fastest when the element type is not Any.

[kahaaga]

I'd expect this to give me a StateSpaceSet whose element type is MVector

julia> mydata = [@MVector randn(rng, 3) for i = 1:5]
5-element Vector{MVector{3, Float64}}:
 [-0.4767453696194501, 0.49546777564614947, 0.17148790479734707]
 [0.8531016875267311, -0.7326026566348486, -0.7576088376271]
 [-0.8980297646429174, 1.5598653293326032, -0.4165139044390249]
 [0.9797013266008067, -0.22808434432221555, -0.3528393361750995]
 [-0.7275549824789735, -1.0126821848114682, -1.7375502156594582]

julia> s = StateSpaceSet(mydata);

julia> typeof(s)
StateSpaceSet{3, Float64, SVector{3, Float64}}

Since the input is automatically converted to SVector, I miss out on mutability (which is the reason for using MVector to begin with).

julia> s[1][2] = 2.0
ERROR: setindex!(::SVector{3, Float64}, value, ::Int) is not defined.
 Hint: Use `MArray` or `SizedArray` to create a mutable static array
Stacktrace:
 [1] error(s::String)
   @ Base ./error.jl:35
 [2] setindex!(a::SVector{3, Float64}, value::Float64, i::Int64)
   @ StaticArrays ~/.julia/packages/StaticArrays/MSJcA/src/indexing.jl:3
 [3] top-level scope
   @ REPL[60]:1

EDIT: Ah, I just realised you can pass the container keyword to the constructor.

However, this also doesn't work, because the constructor doesn't respect the container type - it converts it to a Vector even though I'm specifying that I want the container type to be MVector.

julia> mydata = [@MVector randn(rng, 3) for i = 1:5]
5-element Vector{MVector{3, Float64}}:
 [-0.892115603845179, -0.39542146376334664, -1.6668822864109882]
 [0.78715458665869, -0.022377374505196022, -0.17141201252733881]
 [0.8459705428821778, 0.4679893570831383, 0.9659927748573486]
 [-0.07209551095515201, 0.08674928208016765, 0.3038982127638071]
 [0.7864821827093961, 1.2693614492633645, 0.02604554760068006]

julia> s = StateSpaceSet(mydata; container = MVector{Float64, 5})
3-dimensional StateSpaceSet{Float64} with 5 points
 -0.892116   -0.395421   -1.66688
  0.787155   -0.0223774  -0.171412
  0.845971    0.467989    0.965993
 -0.0720955   0.0867493   0.303898
  0.786482    1.26936     0.0260455

julia> typeof(s)
StateSpaceSet{3, Float64, Vector{Float64}}

[kahaaga]

I'll await further review here until these questions are addressed

[Datseris]

T is supposed to be strictly real. Not even Number, i.e., Compelx numbers not allowed. I'll enforce this in the type.

Sure, MVector could work for the inner vectors, but clearly I haven't coded it correctly yet. The docs say at the moment only SVector and Vector are supported. We can add this (or you can add it if you want). For me this is kinda low priority, and also not necessary to finish this PR.

[kahaaga]

T is supposed to be strictly real. Not even Number, i.e., Compelx numbers not allowed. I'll enforce this in the type.

Ok, sounds good. Tag me when you have something ready, and I'll have a look again.

Sure, MVector could work for the inner vectors, but clearly I haven't coded it correctly yet. The docs say at the moment only SVector and Vector are supported. We can add this (or you can add it if you want). For me this is kinda low priority, and also not necessary to finish this PR.

I'd very much like to support MVectors to facilitate some algorithms that need to modify points of a state space set iteratively without re-creating the vectors. However, given your response, that is beyond the scope of this PR. I can contribute this in a later PR if I get the time to do so.

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 93.93939% with 2 lines in your changes missing coverage. Please review.

Project coverage is 82.63%. Comparing base (593c782) to head (8c96f64).
Report is 8 commits behind head on main.

Files with missing lines	Patch %	Lines
src/statespaceset.jl	91.66%	1 Missing ⚠️
src/statespaceset_concrete.jl	95.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #32      +/-   ##
==========================================
+ Coverage   79.11%   82.63%   +3.51%     
==========================================
  Files           9        9              
  Lines         383      403      +20     
==========================================
+ Hits          303      333      +30     
+ Misses         80       70      -10     

Flag	Coverage Δ
	82.63% <93.93%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Datseris]

Alright, this is done now. MVectors work fine. Keep in mind you need to explicitly set MVector as the container in the constructor. The default is always SVector no matter what the input. This will make this a "non-breaking" change for all of DynamicalSSystems.jl.

[kahaaga]

Alright, this is done now. MVectors work fine. Keep in mind you need to explicitly set MVector as the container in the constructor. The default is always SVector no matter what the input. This will make this a "non-breaking" change for all of DynamicalSSystems.jl.

Great. Thanks!