Skip to content

Conversation

@baggepinnen
Copy link
Member

The function lqr still exists for systems as inputs, where it's clear from the system type which version to call. For matrix inputs, we now require an explicit c/d.
The c/d has been put at the end of the function name instead of in the beginning to be consistent with MatrixEquations and to aid discovery.

lyap still exists without a c in the end since the function belongs to LinearAlgebra.

@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 #563 (1b0b60c) into dev (f644db6) will decrease coverage by 0.10%.
The diff coverage is 86.36%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev     #563      +/-   ##
==========================================
- Coverage   85.08%   84.98%   -0.11%     
==========================================
  Files          31       31              
  Lines        3192     3184       -8     
==========================================
- Hits         2716     2706      -10     
- Misses        476      478       +2     
Impacted Files Coverage Δ
src/ControlSystems.jl 100.00% <ø> (ø)
src/timeresp.jl 89.51% <ø> (-2.16%) ⬇️
src/matrix_comps.jl 88.01% <83.33%> (ø)
src/synthesis.jl 88.46% <83.33%> (ø)
src/simplification.jl 100.00% <100.00%> (ø)
src/types/lqg.jl 83.80% <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...1b0b60c. Read the comment docs.

@imciner2
Copy link
Contributor

Changing dlqr to lqrd will really mess people up who are coming from Matlab. Those two functions in Matlab are very different - dlqr designs the gain assuming a discrete-time system and a discrete-time cost function whereas lqrd designs the gain assuming a continuos-time system and continuous-time cost function (just with both discretized internally, so the discrete-time controller has a cost equivalent to the continuous-time cost). The current implementation matches Matlab's dlqr function, not its lqrd function.

I'm not really a fan of the other changes either, I kind of liked the prefix for differentiating between the continuous and discrete-time versions.

@baggepinnen baggepinnen added the v1 Issues to resolve before releasing v1 label Oct 21, 2021
@baggepinnen baggepinnen deleted the lqrcd branch June 29, 2022 08:33
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.

4 participants