[v10] refactor: change inputs/outputs handling in structural_simplify
#3573
Conversation
vyudu
changed the title
structural_simplify, rename to mtkbuild structural_simplify
test/downstream/linearize.jl
Outdated
| @@ -24,14 +24,14 @@ lsys3, _ = linearize(sys, [r], [y]; autodiff = AutoFiniteDiff()) | |||
| @test lsys.C[] == lsys2.C[] == lsys3.C[] == 1 | |||
| @test lsys.D[] == lsys2.D[] == lsys3.D[] == 0 | |||
|
|
|||
| lsys, ssys = linearize(sys, [r], [r]) | |||
There was a problem hiding this comment.
If we don't return ssys, how can the user know the state realization used? On line 17 above, the ssys is returned, what's different on this line?
There was a problem hiding this comment.
Sorry I forgot to correct these.
Re the first question, my thought was that the user always simplifies it with the inputs first and then passes it into the linearization. So I should add a few simplification statements here first. So they examine the state realization by just examining the simplified system. Is this a reasonable approach?
There was a problem hiding this comment.
In some cases, like when calling linearize, yes, but there are cases when the user cannot simplify and while provide the inputs themselves. The functions that compute sensitivity functions, such as get_sensitivity, start by adding a new input and then simplify with this input specified, the user cannot do that since this input does not even exist before the function was called.
test/downstream/linearize.jl
Outdated
| @@ -114,11 +114,12 @@ Nd = 10 | |||
| @named pid = LimPID(; k, Ti, Td, Nd) | |||
|
|
|||
| @unpack reference, measurement, ctr_output = pid | |||
| lsys0, ssys = linearize(pid, [reference.u, measurement.u], [ctr_output.u]; | |||
| pid = structural_simplify(pid, inputs = [reference.u, measurement.u], outputs = [ctr_output.u]) | |||
| lsys0 = linearize(pid, [reference.u, measurement.u], [ctr_output.u]; | |||
There was a problem hiding this comment.
if the inputs and outputs are passed already on the line above, why do they have to be passed here as well?
There was a problem hiding this comment.
They shouldn't be, thanks. Is it reasonable to default inputs to inputs(sys) and outputs to outputs(sys) if the user does not explicitly pass these in?
|
See discussions here |
|
@AayushSabharwal @baggepinnen reverted the linearization stuff, I think this should be ready |
| @@ -179,9 +179,6 @@ The return values also include the chosen state-realization (the remaining unkno | |||
|
|
|||
| If `disturbance_inputs` is an array of variables, the generated dynamics function will preserve any state and dynamics associated with disturbance inputs, but the disturbance inputs themselves will (by default) not be included as inputs to the generated function. The use case for this is to generate dynamics for state observers that estimate the influence of unmeasured disturbances, and thus require unknown variables for the disturbance model, but without disturbance inputs since the disturbances are not available for measurement. To add an input argument corresponding to the disturbance inputs, either include the disturbance inputs among the control inputs, or set `disturbance_argument=true`, in which case an additional input argument `w` is added to the generated function `(x,u,p,t,w)->rhs`. | |||
|
|
|||
| !!! note "Un-simplified system" | |||
There was a problem hiding this comment.
This warning is still warranted?
There was a problem hiding this comment.
The generate_control_function shouldn't require an un-simplified system anymore since it no longer simplifies the system
|
OK tests should be fixed here. |
Standardize the input handling by no longer returning
input_idxs. To be merged into #3563