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

make feedback differentiable by avoiding try-catch #596

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

baggepinnen
Copy link
Member

Zygote.jl does not handle try-catch, so I modified feedback slightly to avoid using it, but maintained the useful error messagees.

The comment # inv seems to be better than lu made no sense to me since inv is literally implemented like this

Ai = inv!(lu(AA))

@JuliaControlBot
Copy link

Something failed when generating plots. See the log at https://github.com/JuliaControl/ControlExamplePlots.jl/actions/runs/1667112928?check_suite_focus=true for more details.

@codecov
Copy link

codecov bot commented Jan 7, 2022

Codecov Report

Merging #596 (7cb95f6) into master (5c4a565) will increase coverage by 0.05%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #596      +/-   ##
==========================================
+ Coverage   86.64%   86.69%   +0.05%     
==========================================
  Files          31       31              
  Lines        3324     3322       -2     
==========================================
  Hits         2880     2880              
+ Misses        444      442       -2     
Impacted Files Coverage Δ
src/connections.jl 93.90% <100.00%> (+1.13%) ⬆️

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 5c4a565...7cb95f6. Read the comment docs.

@baggepinnen
Copy link
Member Author

Let's put his one on hold, Zygote doesn't actually support @warn either :/

@baggepinnen
Copy link
Member Author

baggepinnen commented Feb 28, 2022

Note to self: hinfnorm fails with both ForwardDiff and ReverseDiff due to GLA's HessenbergFactorization having different fieldnames. ReverseDiff also falis to use the chain rule for hinfnorm due to the TrackedReal appearing in a field of an outer type. and thus RD is failing to pass "tracker free" variables into hinfnorm

typeof(sys) = ControlSystems.StateSpace{Continuous, ReverseDiff.TrackedReal{Float64, Float64, Nothing}}
ERROR: LoadError: type HessenbergFactorization has no field H

typeof(sys) = ControlSystems.StateSpace{Continuous, ForwardDiff.Dual{ForwardDiff.Tag{typeof(cost), Float64}, Float64, 4}}
ERROR: LoadError: type HessenbergFactorization has no field H

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.

None yet

2 participants