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

add option to include disturbance args in generate_control_function #3273

Merged
merged 7 commits into from
Dec 16, 2024

Conversation

baggepinnen
Copy link
Contributor

This PR adds the option to include the disturbance inputs as input arguments for the generated function. This is useful when designing state estimators where the noise model is non-additive.

@AayushSabharwal the PR includes a test, which is currently failing, likely because there is a modification to wrap_array_vars required. wrap_array_vars appears long and complicated without any comments or documentation, what does this function do and can it be made easier to extend?

src/inputoutput.jl Outdated Show resolved Hide resolved
src/inputoutput.jl Outdated Show resolved Hide resolved
Co-authored-by: Aayush Sabharwal <[email protected]>
src/inputoutput.jl Outdated Show resolved Hide resolved
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@baggepinnen
Copy link
Contributor Author

The updates did not make the test pass. Looking at the generated function

julia> f[1]
RuntimeGeneratedFunction(#=in ModelingToolkit=#, #=using ModelingToolkit=#, :((ˍ₋arg1, ˍ₋arg2, ___mtkparameters___, ˍ₋arg4)->begin
          #= /home/fredrikb/.julia/packages/SymbolicUtils/jf8aQ/src/code.jl:385 =#
          #= /home/fredrikb/.julia/packages/SymbolicUtils/jf8aQ/src/code.jl:386 =#
          #= /home/fredrikb/.julia/packages/SymbolicUtils/jf8aQ/src/code.jl:387 =#
          begin
              begin
                  begin
                      begin
                          begin
                              #= /home/fredrikb/.julia/packages/SymbolicUtils/jf8aQ/src/code.jl:480 =#
                              (SymbolicUtils.Code.create_array)(typeof(ˍ₋arg1), nothing, Val{1}(), Val{(1,)}(), (+)((+)(ˍ₋arg2[1], (*)(-1, ˍ₋arg1[1])), (^)(ˍ₋arg4[1], 2)))
                          end
                      end
                  end
              end
          end
      end))

it looks like _arg4 is the disturbance_inputs, but time should have been _arg4 and the disturbance inputs should have been _arg5. Right now, time does not appear to be included among the inputs arguments

@baggepinnen
Copy link
Contributor Author

It turns out that it works correctly with split = false, but test failing with split=true

@AayushSabharwal
Copy link
Member

Ah, right cachesyms does that 😅. I'll push the required changes here.

@baggepinnen
Copy link
Contributor Author

looks good now, the test failures appear to be old

@AayushSabharwal
Copy link
Member

Yeah these failures are "expected". I have plans to fix two of them.

@ChrisRackauckas ChrisRackauckas merged commit 31f7a54 into master Dec 16, 2024
39 of 43 checks passed
@ChrisRackauckas ChrisRackauckas deleted the disturbance_args branch December 16, 2024 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants