write struct_ctrb_states in terms of iszero instead of == 0#557
Merged
baggepinnen merged 2 commits intodevfrom Oct 10, 2021
Merged
write struct_ctrb_states in terms of iszero instead of == 0#557baggepinnen merged 2 commits intodevfrom
struct_ctrb_states in terms of iszero instead of == 0#557baggepinnen merged 2 commits intodevfrom
Conversation
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 ```
|
This is an automated message.
|
Codecov Report
@@ Coverage Diff @@
## dev #557 +/- ##
=======================================
Coverage 85.08% 85.08%
=======================================
Files 31 31
Lines 3192 3192
=======================================
Hits 2716 2716
Misses 476 476
Continue to review full report at Codecov.
|
Contributor
Seems a little bit problematic, but I guess that for ControlSystems.jl, it is mostly in instances like this that it will actually be meaningful to use symbolic variables and checking equality. |
baggepinnen
added a commit
that referenced
this pull request
Nov 7, 2021
* 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 file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The reason is that
iszeroalways returns a bool, whereas== 0may return anything. The difference appears for symbolic variables where