Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove LQG #564

Merged
merged 1 commit into from
Oct 24, 2021
Merged

remove LQG #564

merged 1 commit into from
Oct 24, 2021

Conversation

baggepinnen
Copy link
Member

This PR removes the LQG type. 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

When the version in RobustAndOptimalControl stabilizes, it can be added back to ControlSystems after v1.0 has been released, but we shouldn't include the buggy version we have here when we release v1.0

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
@JuliaControlBot
Copy link

This is an automated message.
Plots were compared to references. 11/11 images have changed, see differences below.
After pulling this PR, please update the reference images by creating a PR to ControlExamplePlots.jl here.

Difference Reference Image New Image
✔️ 0.006 Reference New
❌ 0.048 Reference New
⚠️ 0.016 Reference New
✔️ 0.0 Reference New
✔️ 0.002 Reference New
✔️ 0.002 Reference New
✔️ 0.003 Reference New
✔️ 0.004 Reference New
✔️ 0.012 Reference New
✔️ 0.0 Reference New
✔️ 0.011 Reference New

@codecov
Copy link

codecov bot commented Oct 20, 2021

Codecov Report

Merging #564 (91314d3) into dev (f644db6) will decrease coverage by 0.09%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev     #564      +/-   ##
==========================================
- Coverage   85.08%   84.99%   -0.10%     
==========================================
  Files          31       30       -1     
  Lines        3192     3079     -113     
==========================================
- Hits         2716     2617      -99     
+ Misses        476      462      -14     
Impacted Files Coverage Δ
src/ControlSystems.jl 100.00% <ø> (ø)
src/synthesis.jl 86.53% <ø> (-1.93%) ⬇️
src/timeresp.jl 89.51% <ø> (-2.16%) ⬇️
src/simplification.jl 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a0c960a...91314d3. Read the comment docs.

@baggepinnen baggepinnen added the v1 Issues to resolve before releasing v1 label Oct 21, 2021
@albheim
Copy link
Member

albheim commented Oct 22, 2021

Should we add a note somewhere that LQG is now found in this other package?

@baggepinnen
Copy link
Member Author

Yes, we should make a note of all breaking changes in the release notes. I've started tagging issues and prs with the label v1, we can go through all those when we write the release notes.

@olof3
Copy link
Contributor

olof3 commented Oct 24, 2021

Yes, good to get rid of this. Not sure if we really need a type for this, but we can think about that later.

@baggepinnen baggepinnen merged commit 57846d1 into dev Oct 24, 2021
@baggepinnen baggepinnen deleted the noLQG branch October 24, 2021 18:00
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v1 Issues to resolve before releasing v1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants