fix type errors in ss #625
Conversation
the division might change the type, causing the `ST` constructor to fail
src/types/StateSpace.jl
Outdated
|
|
||
| Base.inv(sys::AbstractStateSpace) = 1/sys | ||
| /(sys::ST, n::Number) where ST <: AbstractStateSpace = ST(sys.A, sys.B, sys.C/n, sys.D/n, sys.timeevol) | ||
| /(sys::ST, n::Number) where ST <: AbstractStateSpace = ss(sys.A, sys.B, sys.C/n, sys.D/n, sys.timeevol) |
There was a problem hiding this comment.
Does this work with heterostatespace then?
There was a problem hiding this comment.
nope, hss needs it's own overload. For Int it didn't work before either since hss is even stricter with the types, but this actually breaks hss for float types :/, good catch. It's a bit unfortunate that we can't use ST here, perhaps something like constructor(ST) that strips the type parameters would be nice to have to make it generic
There was a problem hiding this comment.
or, hss will still work, but "demote" to standard ss
There was a problem hiding this comment.
perhaps something like constructor(ST) that strips the type parameters
Is this easy to do? Remember I tried to do something similar with maybe c2d for tf, and we wanted to return a tf of same type as in zpk/rational but the number types could change and timeevol would change. I think I ended up just dispatching differently because I only found some internal methods to access that data which didn't seems stable, but might have changed (or I might not have found the best method).
There was a problem hiding this comment.
This
julia> tG = typeof(G)
TransferFunction{Continuous, ControlSystems.SisoRational{Int64}}
julia> tG.name.wrapper
TransferFunctionis probably one of the most used internals of julia since there is no "supported" way of doing it, but it's often very useful to do. The alternative would be that every type of LTISystem implements a method like
constructor(::StateSpace) = StateSpace
(without the type parameters). This also works, but it's a bit unfortunate that it's required. There's an issue about exposing .name.wrapper as an API and the consensus appears to be that it should be done, but no one has done it yet.
JuliaLang/julia#35543
I think we could introduce our own basetype function until one exists in Base.
|
This is an automated message.
|
Codecov Report
@@ Coverage Diff @@
## master #625 +/- ##
=======================================
Coverage 86.75% 86.75%
=======================================
Files 31 31
Lines 3253 3217 -36
=======================================
- Hits 2822 2791 -31
+ Misses 431 426 -5
Continue to review full report at Codecov.
|
and use it to make statespace code simpler
the division might change the type, causing the
STconstructor to fail.Closes #299