-
Notifications
You must be signed in to change notification settings - Fork 6
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
Force positive definite Gramiam matrices when balancing #119
base: master
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## master #119 +/- ##
==========================================
- Coverage 100% 99.69% -0.31%
==========================================
Files 28 28
Lines 1321 1331 +10
Branches 155 156 +1
==========================================
+ Hits 1321 1327 +6
- Misses 0 3 +3
- Partials 0 1 +1
Continue to review full report at Codecov.
|
This may not be the correct solution. On a different machine, higher-order (e.g., 60) Pade delays have Gramiam matrices with extremely large negative eigenvalues (e.g., -1e6). These numerical issues require a more careful examination. In the mean time throwing a numerical exception seems reasonable (although admittedly not user-friendly). |
WIP: Investigating possible differences between home machine and lab machine:
If the matrices are the same then the issue might be in the solver. |
Matrices are the same (within numerical epsilon) on my lab machine, but still getting different results. Need to check solver results somehow, or something else entirely... Using same version of |
This might be avoided altogether by the use of the |
Needs unit test.
For whatever reason switching my BLAS had the side-effect of introducing rounding errors into the observability Gramiam which makes the Cholesky decomposition raise
LinAlgError: *-th leading minor not positive definite
. Specifically this occurred when balancing a 27-dimensional PadeDelay system.The approach taken here is to perturb the eigenvalues by the smallest possible value such that they all become positive. I've arbitrarily chosen
1e-19 - eig
as the largest possible regularization, whereeig
is the smallest eigenvalue that is assumed to be no smaller than-1e-9
. The logic is that if the smallest eigenvalue is any smaller than this, then something must be very wrong (and is then made to fail here). The1e-19
is an epsilon needed to make the eigenvalues positive (rather than non-negative). These constants may need to be tweaked or exposed. Tested manually.