JuliaControl / ControlSystems.jl

A Control Systems Toolbox for Julia
https://juliacontrol.github.io/ControlSystems.jl/stable/
Other
513 stars 85 forks source link

Round transfer-function terms, if only for printing #382

Open KronosTheLate opened 4 years ago

KronosTheLate commented 4 years ago

When I enter some parameter values into a complicated expression (with multiple fractions, often nested and multiplied with each other), the resulting transfer function will often have coefficients like 4.99999999999999 with something like 12 digits. This makes visual inspection and comparisons of the resulting transfer function very annoying to deal with, and it seems like no benefit is to be gained - in most cases, if one works through it symbolically, the rounded value the actual value. I would like to suggest that each term in transfer functions is rounded off after something like the 10th decimal. I believe that this would fix the problem as described in many cases, without rounding being aggressive enough it causes problems.

I don't think I have the know-how to make a good implementation of this, and I want to check if others agree before I make an attempt at some implementation, so I am only inviting discussion here for now.

baggepinnen commented 4 years ago

I agree that it can be a bit annoying woth too many digits displayed, but there is something important to consider here: the poles of high order polynomials are extremely sensitive to rounding in the polynomial coefficients. Someone copying the coefficients of a transfer function from the repl might then get a very different polynomial than what the transfer function actually represents. This fact makes automatic rounding feel like a foot gun.

This problem also applies to discrete time systems with fast sampling, if we round the coefficients of those, a slow pole will turn into an integrator.

KronosTheLate commented 4 years ago

The points you raise seem more important than the convenience added by rounding. Would a solution be to add a method to round that takes a transfer function and rounds it to some optional second argument integer? Then, the docstring could warn about using the rounded version for anything beyond visual inspection.

If you don't have a problem with that suggestion, I would love to make a PR with the implementation. In that case, a pointer at how one could extract the coefficients of a transfer function would be very helpful.

olof3 commented 4 years ago

I think I would prefer if the coefficients were shown with less excessive precision, (compare for example with how row vectors are shown). I don't think anyone would like to have the elements in a row vectors shown to full precision, and I guess one has to assume. Transfer function representations are notoriously sensitive, so if this is a concern I guess one should stay away from them in the first place.

This problem also applies to discrete time systems with fast sampling, if we round the coefficients of those, a slow pole will turn into an integrator.

Yes, this is a problem. Perhaps we should treat such cases a bit more carefully, i.e., higher precision for systems with fast sampling rates?

I actually had some discussion over this at the Polynomials package quite recently, when this excessive precision was brought up in another issue. The discussion was perhaps somewhat confused, but it ended up with a PR that added a rounding option in printpoly. Hopefully that option could be used do what you want I would start looking there.

baggepinnen commented 4 years ago

Yes, this is a problem. Perhaps we should treat such cases a bit more carefully, i.e., higher precision for systems with fast sampling rates?

If we can figure out a robust way of doing this then I guess it could be a good idea. Ideally, we should have some bound on how much the rounding is allowed to move the poles, or perhaps how much it is allowed to change the frequency response of the system. One can figure out the gradient of the residue of a pole w.r.t the coefficients and make sure the rounding does not change the residue too much, with the residue being a proxy for how much energy the pole contributes to the spectrum. Poles very near the imaginary axis, or unit circle in the discrete domain, has a very large residue gradient w.r.t. the coefficients.

KronosTheLate commented 4 years ago

If we can figure out a robust way of doing this then I guess it could be a good idea. Ideally, we should have some bound on how much the rounding is allowed to move the poles, or perhaps how much it is allowed to change the frequency response of the system. One can figure out the gradient of the residue of a pole w.r.t the coefficients and make sure the rounding does not change the residue too much, with the residue being a proxy for how much energy the pole contributes to the spectrum. Poles very near the imaginary axis, or unit circle in the discrete domain, has a very large residue gradient w.r.t. the coefficients.

I only understand parts of this, but if one could make the rounding such that the changes in the characteristics of the transfer functions are minimal, that sounds like a pretty much perfect solution as far as I am concerned.

mfalt commented 4 years ago

I don't think we should do any rounding i of the elements internally. But it could be a reasonable option to print the systems in a prettier way. I don't think users expect that the printed decimals are exact in general, but maybe I'm wrong.

olof3 commented 4 years ago

I don't think users expect that the printed decimals are exact in general, but maybe I'm wrong.

Yeah, we should be able to assume this. I guess the full precision should be obtained using show with certain arguments, but the default output to the REPL should be more compact.

For discrete-time transfer functions of very low order (one, possibly two) we could perhaps use higher precision for rapidly sampled discrete-time systems, but perhaps it is primarily when printing zpk models that we should have a higher precision.

KronosTheLate commented 4 years ago

In terms of implementation, I came over something possibly relevant in the Polynomials.jl documentation (https://github.com/JuliaMath/Polynomials.jl):

"chop chops off any small leading values that may arise due to floating-point operations."

If transfer functions are implemented using Polynomials.jl as I believe, this implementation should be pretty straight forward. Or, if only inspiration is needed, the implementation is found under "https://github.com/JuliaMath/Polynomials.jl/blob/master/src/common.jl"

mfalt commented 4 years ago

In terms of implementation, I came over something possibly relevant in the Polynomials.jl documentation (https://github.com/JuliaMath/Polynomials.jl):

"chop chops off any small leading values that may arise due to floating-point operations."

If transfer functions are implemented using Polynomials.jl as I believe, this implementation should be pretty straight forward. Or, if only inspiration is needed, the implementation is found under "https://github.com/JuliaMath/Polynomials.jl/blob/master/src/common.jl"

It seems that this implementation is not cutting trailing digits in the coefficients, but rather cutting away coefficients that are small, I don't think that's the major problem here. Anyway, I think the implementation would be fairly straight forward once we decide how we want it to behave.

Edit: I think calling printpoly with compact=true will be sufficient. This is implemented as an argument in polynomials and is used for the coefficients. compact=true is used in for example Base to print the elements in a matrix to reduce the number of digits.