Linear model #6
Linear model #6
Conversation
…ab/RobotDynamics.jl into linear_model new changes from master
There was a problem hiding this comment.
Nice work so far. I mentioned this a couple places, but there seems to be a lot of repetitive code in here, which makes it hard to maintain and debug. I'd like to clean it up a little bit. Julia is usually pretty good at making a lot out a few lines of code. Let's think a bit more about how we can clean it up and make it more unified. I'm also not in love with the interface yet. Having separate macros for time invariant and time-varying for example: we should have one and just pass a is_time_varying flag to the macro, like we do for is_affine.
|
|
||
| Creates a struct that looks like: | ||
| ```julia | ||
| struct (name){TA, TB, Td} <: ContinuousLTI |
There was a problem hiding this comment.
Are you using RefValue to allow the values to change? I'm curious if this is any more efficient than just making the struct mutable? I think I'd prefer to make the struct mutable since it'll probably make it easier to understand what's going on.
|
|
||
| return (A_d, B_d) | ||
| function _ltv_affine(name, n, m, N, supertype) |
There was a problem hiding this comment.
There's a lot of redundant code in here. For example, I think it would be pretty easy to combine the affine and non-affine functions in one. Just have a little bit of branching for the few things that are different rather than copying a bunch of code. It makes it easier to maintain and reduces bloat.
| @@ -0,0 +1,197 @@ | |||
| """ | |||
There was a problem hiding this comment.
General comment here: this file seems overly verbose. There's a lot of repeated code. Is there a way we can clean this up to make it easier to understand and maintain?
| _discretize!(::Type{Q}, ::Val{true}, discrete_model::DiscreteLinearModel, continuous_model::ContinuousLinearModel, k::Integer, dt) where {Q<:Explicit} = | ||
| _discretize!(Q, Val(true), discrete_model, get_A(continuous_model, k), get_B(continuous_model, k), get_d(continuous_model, k), k, dt) | ||
|
|
||
| function _discretize!(::Type{Exponential}, ::Val{true}, discrete_model::DiscreteLinearModel, A::AbstractMatrix, B::AbstractMatrix, d::AbstractVector, k::Integer, dt) |
There was a problem hiding this comment.
I feel these discretization schemes are tangential to the ones in integration.jl. I'd like to have one place of integration schemes, rather than placing them in different places and without having them be interchangeable. Can we unify the ways these are used? For example, I should be able to use RK4 with discrete_jacobian or discretize.
The remaining items to do include: