`isapprox` for `SisoRational` broken

[baggepinnen]

julia> using ControlSystems

julia> G1  = tf([0.8], [1,0,0.25], 1)

TransferFunction{Discrete{Int64}, ControlSystems.SisoRational{Float64}}
     0.8
-------------
1.0z^2 + 0.25

Sample Time: 1 (seconds)
Discrete-time transfer function model

julia> G2  = tf([0.8, 0], [1,0,0.25], 1)
TransferFunction{Discrete{Int64}, ControlSystems.SisoRational{Float64}}
    0.8z
-------------
1.0z^2 + 0.25

Sample Time: 1 (seconds)
Discrete-time transfer function model

julia> isapprox(G1, G2)
true

note the different orders of the numerators
ref baggepinnen/ControlSystemIdentification.jl#75 (comment)

[olof3]

The issue seems to be a poor way to ensure that B(z)/A(z) = cB(z)/(cA(z)) for som constant c.

function isapprox(f1::SisoRational, f2::SisoRational; rtol::Real=sqrt(eps()), atol::Real=0)
    isapprox(f1.num * f2.den[1], f2.num * f1.den[1], rtol=rtol, atol=atol) && isapprox(f1.den * f2.den[1], f2.den * f1.den[1], rtol=rtol, atol=atol)
end

In this case the first order term in the denominator is zero, so everything becomes zero. Not sure how to handle this case properly? Finding the largest coefficient in the denominator and use that for scaling seems like a straightforward fix, but it does not seem ideal.

More generally, I guess it would be good if cancellations, etc, also were considered. I think we have this problem quite generally for systems that can have different realizations. One approach would be to convert to state space and consider the L2 or Linf norm of the difference between the two systems.

[albheim]

Fixed by #622