Conversation
|
This is an automated message.
|
|
This is an automated message.
|
|
This is an automated message.
|
Codecov Report
@@ Coverage Diff @@
## dev #536 +/- ##
======================================
Coverage ? 84.92%
======================================
Files ? 31
Lines ? 3197
Branches ? 0
======================================
Hits ? 2715
Misses ? 482
Partials ? 0 Continue to review full report at Codecov.
|
|
Since formatting and style are subjective, it would be nice if someone with a keen sense of style, like @mfalt , would weigh in :) |
|
I can say that for the example you provided I prefer the updated look at least. |
|
Definitely better with the updated look, at least for the considered system. (an extremely poorly damped system, but with time-scales on the order of seconds). I guess one should also consider the formatting for systems that both has dynamics on this time scale as well as dynamics that is much faster / slower. Anyway, here are some other suggestions
|
|
Your output is definetely easier to read, and if I understand it correctly will make no difference compared to before when the exponents are large/small. So I have no objections. But I have never used dampreport either :) |
Those are all good suggestions, especially the pm operator would be nice to use for real-coefficient systems. I was actually wondering about the time constants in this example, is the way we're calculating them always valid no matter the total order of the system? Wn, zeta, ps = damp(sys)
t_const = 1 ./ (Wn.*zeta) |
As the "time constants of the poles" it seems to be alright, I think. There is the concept of average residence time (see e.g. Advanced PID Control) which is primarily used for stable not-very-resonant industrial processes. That could be useful to print out as well, but not sure to what extent to do it for resonant systems, etc. |
|
I made some further updates, we now have three cases, complex coeff systems is the most general, it prints all poles. Real coeff system with complex poles print using the ± operator, and real poles print using no ± operator. The formatting for complex-coeff systems is perhaps not super optimal, but I got quite tired of trying and it should be better than before julia> dampreport(ex_4)
| Pole | Damping | Frequency | Frequency | Time Constant |
| | Ratio | (rad/sec) | (Hz) | (sec) |
+--------------------+---------------+---------------+---------------+---------------+
| +0 | -1 | 0 | 0 | -Inf |
| -0.0597 ± 0.0171im | 0.961 | 0.0621 | 0.00989 | 16.7 |
| -0.0858 | 1 | 0.0858 | 0.0137 | 11.7 |
| -0.18 | 1 | 0.18 | 0.0287 | 5.55 |
julia> dampreport(1/(s+1+2im)/(s+2+3im))
| Pole | Damping | Frequency | Frequency | Time Constant |
| | Ratio | (rad/sec) | (Hz) | (sec) |
+--------------------+---------------+---------------+---------------+---------------+
| -1 -2im | 0.447 | 2.24 | 0.356 | 1 |
| -2 -3im | 0.555 | 3.61 | 0.574 | 0.5 |
julia> dampreport(sys)
| Pole | Damping | Frequency | Frequency | Time Constant |
| | Ratio | (rad/sec) | (Hz) | (sec) |
+--------------------+---------------+---------------+---------------+---------------+
| -1 | 1 | 1 | 0.159 | 1 |
| +4 | -1 | 4 | 0.637 | -0.25 |
| -4 | 1 | 4 | 0.637 | 0.25 | |
|
Great improvements, it looks pretty good now. One could consider switching the columns of frequency (rad/sec) and the damping ratio, but it is also nice that the frequency information is kept together. |
* Avoid unnecessarily large realization for feedback of TransferFunction (#485) * Avoid unnecessarily large realization for feedback of TransferFunction * Fix and added more tests. * Change to numpoly, denpoly * add dev brach to PR CI * Switch u layout (#480) * switch u layout for lsim * Update src/timeresp.jl Co-authored-by: Fredrik Bagge Carlson <baggepinnen@gmail.com> * More updates, one error in test_timeresp * Fix tests * Change to AbstractVecOrMat * Catch CuArray in matrix conversion * General zero vectors for x0 to support GPUs * Update src/timeresp.jl Co-authored-by: Mattias Fält <mfalt@users.noreply.github.com> * Update src/timeresp.jl Co-authored-by: Mattias Fält <mfalt@users.noreply.github.com> * Move f outside lsim * Remove GPU compatible x0, save for later * Fix doctest * add kwargs * Remove variable and generalize type Co-authored-by: Fredrik Bagge Carlson <baggepinnen@gmail.com> Co-authored-by: Mattias Fält <mfalt@users.noreply.github.com> * Gangof4 fixes (#486) * QOL improvements for plotting * remove spurious getindex * update docstring * multiple Ms in nyquist * bugfixes * make rings appear in all subplots * Minor fixes to gang-of-four functionality. * Fixes to Nyquist plots. * Updated the nyquistplot docstring Co-authored-by: Fredrik Bagge Carlson <baggepinnen@gmail.com> * add frequency in Hz to dampreport (#488) * updates to nyquistplot (#493) * updates to nyquistplot * Update src/plotting.jl Co-authored-by: olof3 <olof3@users.noreply.github.com> * Update src/plotting.jl Co-authored-by: olof3 <olof3@users.noreply.github.com> Co-authored-by: olof3 <olof3@users.noreply.github.com> * fix traces in rlocus (#491) * fix traces in rlocus * Update src/pid_design.jl Co-authored-by: Albin Heimerson <albin.heimerson@control.lth.se> Co-authored-by: Albin Heimerson <albin.heimerson@control.lth.se> * let lsim handle arguments in lsimplot (#492) * bugfix: avoid creating continuous system with Ts=0.0 (#496) * Deactivate _preprocess_for_freqresp (#497) until hessenberg is properly used * allow balance when converting tf to ss (#495) * allow balance when converting tf to ss * use zeros(T) instead of fill(zero(T)) * Update src/types/StateSpace.jl Co-authored-by: olof3 <olof3@users.noreply.github.com> * Update src/types/conversion.jl Co-authored-by: Albin Heimerson <albin.heimerson@control.lth.se> Co-authored-by: olof3 <olof3@users.noreply.github.com> Co-authored-by: Albin Heimerson <albin.heimerson@control.lth.se> * Better handling of problematic cases for delay systems (#482) * delay error should be correct now * warn limit * Add tustin c2d/d2c method (#487) * add Tustin discretization * no indexing after c2d * implement f_prewarp in tustin and test vs. matlab * fixe use wrong Ts * f_prewarp -> w_prewarp * w_prewarp in tests * correct handling of x0 in lsimplot (#498) * move eye def to framework.jl (#499) * Fix UndefVarError: T not defined (#501) * add axes for ss (#504) * pi place and tests (#502) * pi place and tests * Fix test * Add ending space in file * Update numvec denvec * Fixed error * Update test/test_pid_design.jl Co-authored-by: olof3 <olof3@users.noreply.github.com> * Update test/test_pid_design.jl Co-authored-by: olof3 <olof3@users.noreply.github.com> * Update forms * Fix test * Fix test * Update src/pid_design.jl Co-authored-by: olof3 <olof3@users.noreply.github.com> * Update src/pid_design.jl Co-authored-by: olof3 <olof3@users.noreply.github.com> * Update src/pid_design.jl Co-authored-by: olof3 <olof3@users.noreply.github.com> Co-authored-by: olof3 <olof3@users.noreply.github.com> * Conver to tf in placepi (#507) * Use nstates instead of nx in lsimplot (#508) * Use nstates instead of nx in lsimplot * Add predictor (#511) * up innovation_form and add noise_model * keep innovation_form, add predictor * export predictor * add hats * updates to `obsv` (#517) * updates to `obsv` All computing an arbitrary number of rows in the observability matrix and accept `AbstractStateSpace` * Update src/matrix_comps.jl Co-authored-by: olof3 <olof3@users.noreply.github.com> Co-authored-by: olof3 <olof3@users.noreply.github.com> * `hz` keyword in `nyquistplot` similar to in `bodeplot` (#518) * Fixes to place (#500) * Add tests for place. * Removed luenberger and exented place instead. Co-authored-by: Fredrik Bagge Carlson <baggepinnen@gmail.com> * allow AbstractStateSpace in several places (#520) * allow AbstractStateSpace in several places * Maintain type for zpk in c2d (#522) * maintain zpk type in c2d * Fix type in typeconversion for c2d tf * Fix type in c2d tf/zpk * Remove assertion on tf/zpk SISO for c2d * Test for c2d(zpk(..)..) unbroken * Keep MIMO assertion c2d tf * Add comment and fix test * Fix test * h to Ts * Fix test * remove assert * split into methods * Remove asserts (#523) * Replace asserts * add error types * fix conversion for custom types (#514) * fix conversion for custom types * special numeric_type for AbstractSS * fix conversion from ss to tf without type * more abstract statespaces (#521) * omre abstract statespaces * even more ASS * remove simple feedback in favor of mor egeneral version * propagate timeevol * add block diagram to feedback docstring * Updates to docstring * fix docstring formatting * delete redundancy in feedback docstring Co-authored-by: olof3 <olof3@users.noreply.github.com> * add controller function (#512) * add controller function * rename controller and predictor * Move plot globals to runtests (#531) * Move plot globals to runtests * Move plot globals to framework.jl * return similarity transform from `balreal` (#530) * display error when covar fails * return similarity transform from balreal * Update src/matrix_comps.jl Co-authored-by: olof3 <olof3@users.noreply.github.com> Co-authored-by: olof3 <olof3@users.noreply.github.com> * remove incorrect warning in pzmap (#535) * support hz keyword in sigmaplot similar to bodeplot (#537) * change formatting in dampreport (#536) * change formatting in dampreport * update dampreport to use ± * even better formatting * up formatting for complex systems * add additive identity element for statespace and TF (#538) * add additive identity element for statespace and TF * rm zero from type. Test MIMO zero * Fix struct_ctrb_obsv (#540) * Fix struct_ctrb_obsv, closes #409 * Update src/simplification.jl Co-authored-by: Fredrik Bagge Carlson <baggepinnen@gmail.com> * add to docstring Co-authored-by: olof3 <olof3@users.noreply.github.com> * Yet another fix for struct_ctrb_states. Closes #475 (#541) * bugfix for negative real pole in damp (#539) * Fix #546 * minor typographic changes * optional epsilon in dcgain (#548) * optional epsilon in dcgain * Update analysis.jl * bugfix: use correct type for saving dde solutions (#549) Would be good to have a regression test for `BigFloat` data which was the reason I found this bug. That seems slightly involved to fix though. I assume that this would also have failed for `ComplexF64`, so eventually a test for that too would be good. * bugfix dcgain (#551) * correct type of initial state in step (#553) * remove version checks * Fix spacing in type printing * write `struct_ctrb_states` in terms of `iszero` instead of `== 0` (#557) * write `struct_ctrb_states` in terms of `iszero` instead of `== 0` The reason is that `iszero` always returns a bool, whereas `== 0` may return anything. The difference appears for symbolic variables where ```julia julia> q0 == 0 q0(t) == 0 julia> iszero(q0) false ``` * Update simplification.jl * remove `issmooth` (#561) * remove issmooth * drop extra `]` * Return a result structure from lsim (#529) * Return a result structure from lsim This is by design a non-breaking change to the lsim interface since the structure allows both getindex and destructuring to behave just like if lsim returned a tuple of arrays like before. Indeed, the tests for lsim were not touched in this commit. This commit also adds a plot recipe to the result structure. All plot recipes for lsimplot, stepplot, impulseplot have been replaced by the new recipe. This is a breaking change since the names of the previous plots no longer exist. A slight change is that the plots for a step response no longer show the text "step response", but I think that's an acceptable change, the user can supply any title they prefer themselves. * remove automatic title * introduce additional abstract result type * do not destructure sys * remove LQG (#564) The functionality was very buggy and poorly tested. A much improved version with proper tests are available in https://github.com/JuliaControl/RobustAndOptimalControl.jl/blob/master/src/lqg.jl * pole->poles tzero->tzeros (#562) * update usage of plot for step simulation * add doctest filters * add release notes * Update README.md * Update README.md Co-authored-by: olof3 <olof3@users.noreply.github.com> Co-authored-by: Albin Heimerson <albin.heimerson@control.lth.se> Co-authored-by: Mattias Fält <mfalt@users.noreply.github.com>
This PR changes the formatting flag from
etogin dampreport.gmakes the formatting adaptive, choosing a reasonable way to format the number based on its value.From wikipedia https://en.wikipedia.org/wiki/Printf_format_string