debug: remove .data in KalmanFilter matrix products
#189
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some matrix products were computed on normal dense matrix with `M.data` instead of `Hermitian`. It was because of a bug in `LinearAlgebra` that created a dynamic dispatch. This bug is resolved now so it's no longer needed.
|
Amazing, thanks for the help! The example works on my computer now. |
Member
Author
|
P.s. the CI fails but everything pass locally on my side. I'm investigating on this. |
franckgaga
referenced
this pull request
in JuliaControl/ControlSystems.jl
Apr 18, 2025
also decreases the tolerances since computation are more exact than before
Member
Author
|
The next release will incorporate this change. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #189 +/- ##
=======================================
Coverage 98.88% 98.88%
=======================================
Files 25 25
Lines 4205 4205
=======================================
Hits 4158 4158
Misses 47 47 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Some matrix products were computed on normal dense matrix using
M.datainstead ofM, in whichM isa Hermitian. It was because of a bug inLinearAlgebrathat produces a dynamic dispatch (JuliaLang/julia#53951). This bug is solved now so it's no longer needed.It's also not the best idea for the numerical stability since covariance matrices are
Hermitians, and the intermediate matrices in the computation should all be Hermitians also for the consistency.@1-Bart-1 This branch solve the
cholesky!issue in your exemple on my computer. Give it a try.I did not try the Joseph form equation here since the
.datawas just bad for the numerical stability, and simply removing that solve the problem. After I merge these changes, If you have any other similar issue with a new piece of code, send it to me. I will try the Joseph form on it to see if it helps.related to #98