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

A bug? #365

Open
andreasvarga opened this issue Nov 7, 2021 · 6 comments
Open

A bug? #365

andreasvarga opened this issue Nov 7, 2021 · 6 comments

Comments

@andreasvarga
Copy link

I started some cleaning of my tests for rational transfer functions. The following fails and was commented out for RationalTransferFunctions. It seems however, it doesn't work for RationalFunctions too.

julia> using LinearAlgebra

julia> using Polynomials

julia> s = RationalFunction(Polynomial([0,1],:s))
(s) // (1)

julia> [s^2 s/(s+1); I]
ERROR: ArgumentError: number of columns of each array must match (got (2, 1))
Stacktrace:
 [1] _typed_vcat(#unused#::Type{Any}, A::Tuple{Matrix{Any}, Matrix{Any}})
   @ Base .\abstractarray.jl:1553
 [2] typed_vcat(::Type{Any}, ::Matrix{Any}, ::Matrix{Any})
   @ Base .\abstractarray.jl:1567
 [3] typed_hvcat(::Type{Any}, ::Tuple{Int64, Int64}, ::RationalFunction{Int64, :s, Polynomial{Int64, :s}}, ::Vararg{Any, N} where N)
   @ Base .\abstractarray.jl:1957
 [4] hvcat(::Tuple{Int64, Int64}, ::RationalFunction{Int64, :s, Polynomial{Int64, :s}}, ::Vararg{Any, N} where N)
   @ Base .\abstractarray.jl:1932
 [5] top-level scope
   @ REPL[4]:1
@jverzani
Copy link
Member

jverzani commented Nov 7, 2021

I'm thinking I really wants to be a square matrix:

julia> using LinearAlgebra

julia> [1 2; I]
ERROR: ArgumentError: number of columns of each array must match (got (2, 1))

This works, but ends up with an odd type:

julia> [s^2 s/(s+1); 0  I]
2×2 Matrix{Any}:
  (s^2) // (1)  (s) // (1 + s)
 0              UniformScaling{Bool}(true)

but the same is true if s is just a number.

@andreasvarga
Copy link
Author

Interesting. Thus, this is an error in LinearAlgebra. Should I make an issue for this?

Recently I fighted with type piracy in DescriptorSystems. I was not able to get rid of it, but enhanced my handling of such cases (e.g., by internally converting the above to [[1] [2]; I], which works). Now I realize, this was not entirely my fault!

@jverzani
Copy link
Member

jverzani commented Nov 7, 2021 via email

@andreasvarga
Copy link
Author

Only the square case is implemented in LinearAlgebra.UniformScaling at line #394:
promote_to_arrays_(n::Int, ::Type{Matrix}, J::UniformScaling{T}) where {T} = copyto!(Matrix{T}(undef, n,n), J)

With my handling of type piracy I get:

julia> using DescriptorSystems
[ Info: Precompiling DescriptorSystems [a81e2ce2-54d1-11eb-2c75-db236b00f339]


julia> [1 2; I]
3×2 Matrix{Int64}:
 1  2
 1  0
 0  1

I would be happy to get the same with LinearAlgebra too.

@jverzani
Copy link
Member

jverzani commented Nov 7, 2021 via email

@andreasvarga
Copy link
Author

Apparently we have to wait for Julia 1.8 to be fixed in LinearAlgebra. Should we close this issue?

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

No branches or pull requests

2 participants