Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Removed epsilon perturbation value in solve_passivity_LMI. Fix associated unit test.#814

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
murrayrm merged 3 commits intopython-control:mainfromMark-Yeatman:main
Dec 17, 2022

Conversation

Mark-Yeatman
Copy link
Contributor

@Mark-YeatmanMark-Yeatman commentedDec 12, 2022
edited
Loading

This should hopefully address#761

There was a unit test that was supposed to "hint at" how much you could scale matrices and expect to still correct results. It was marked as xfail after it was flakey on githubs integration pipeline. Hopefully this change removes the flakeyness.

This change also pushes more responsibility on the user for knowing if their system is "well conditioned". I suspect the root cause of the flakeyness is that CVXOPT does matrix inversions while solving the passivity LMI, and the algorithm would divide by zero if the condition numbers of the system matrices were too high. So, I added a exception handling case to catch the zero division during the cvxopt solve, said we think its because the matrices are ill conditioned, and then reraised the exception.

I then empirically bumped down the relative scaling on the unit test until it started passing on my local system.

…t test to reflect scaling values from empirical testing.
@coveralls
Copy link

coveralls commentedDec 12, 2022
edited
Loading

Coverage Status

Coverage increased (+0.008%) to 94.81% when pullinge6b837d on Mark-Yeatman:main into9d65bf8 on python-control:main.

@bnavigator
Copy link
Contributor

I then empirically bumped down the relative scaling on the unit test until it started passing on my local system.

This is dangerous. There is a zoo of lower precision hardware systems out there where python-control might be used and packaged. For example, I frequently encounter errors for such tests on the openSUSE build system with i586, aarch64, 32-bit arm, s390x, PowerPC and so on platforms.

@Mark-Yeatman
Copy link
ContributorAuthor

Mark-Yeatman commentedDec 14, 2022
edited
Loading

I think we'd either have to delete this test then; or do something like detect the precision of the architecture somehow, then back out the values of the matrices such that the condition number is bounded by like 10^(n/2), where n is the number of bits allocated to representing floating points numbers. (Just spit balling).

bnavigator reacted with thumbs up emoji

@Mark-Yeatman
Copy link
ContributorAuthor

I will delete the test.

@@ -14,7 +14,6 @@
except ImportError:
cvx = None

eps = np.nextafter(0, 1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

(This is a by-the-way comment, feel free to ignore.)

Thiseps wasvery small; I think a more typical "eps" would be "the smallest number eps such that 1+eps > 1", or a multiple or maybe square root of that.np.finfo is useful to find these sorts of value.

In [402]: np.nextafter(0, 1)Out[402]: 5e-324In [403]: np.finfo(float).epsOut[403]: 2.220446049250313e-16

bnavigator reacted with thumbs up emoji
Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

That is a pro tip, and what I was aiming for.

@murrayrmmurrayrm linked an issueDec 16, 2022 that may beclosed by this pull request
Co-authored-by: Ben Greiner <code@bnavigator.de>
@murrayrmmurrayrm merged commit2d0d513 intopython-control:mainDec 17, 2022
@murrayrmmurrayrm added this to the0.9.3 milestoneDec 24, 2022
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@roryyorkeroryyorkeroryyorke left review comments

@bnavigatorbnavigatorbnavigator left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.9.3
Development

Successfully merging this pull request may close these issues.

Passivity test is ill-conditioned
5 participants
@Mark-Yeatman@coveralls@bnavigator@roryyorke@murrayrm

[8]ページ先頭

©2009-2025 Movatter.jp