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

Add passivity module, ispassive function, and passivity_test. Introduces optional dependency cvxopt.#739

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 21 commits intopython-control:mainfromMark-Yeatman:passivity-tools
Jun 16, 2022

Conversation

Mark-Yeatman
Copy link
Contributor

I made a discussion topic a couple of weeks ago about making a passivity module,@murrayrm suggested making a draft PR. Here it is!

@Mark-YeatmanMark-Yeatman marked this pull request as ready for reviewMay 31, 2022 03:53
@Mark-Yeatman
Copy link
ContributorAuthor

I need to cut the formation of the LMI via symbolic variables for performance. Did a timing test on a size 20 state, input, output system, 99% of the time is spent constructing the LMI and converting to cvxopt form.

@bnavigator
Copy link
Contributor

bnavigator commentedMay 31, 2022
edited
Loading

The initial disussion is here:#732

Thanks for the initiative!

As far as I understand, you introduce 3 new depenendies because you work with symbolic matrices:

Would it be possible to implement the matrix handling and solving with NumPy and SciPy functions only?

@Mark-Yeatman
Copy link
ContributorAuthor

Would it be possible to implement the matrix handling and solving with NumPy and SciPy functions only?

I'm about 60% sure cvxopt is necessary to solve the linear matrix inequality. Sympy and lmi_sdp can definitely be dropped.

Cvxopt only requires BLAS/LAPACK, which it seems that slycot also requires. So this passivity package could be setup to be optional?

@bnavigator
Copy link
Contributor

CVXOPT is available on conda-forge and as wheels with bundled libraries on PyPI. They link and bundle OpenBLAS. I am not sure how well that plays with "Generic" Netlib linked NumPy and Slycot packages. But we can worry about that when it comes up.

It must be installed in the test suite, so we need to add it to the requirements. It can be set as optional similar to how we do it with Slycot.

@Mark-Yeatman
Copy link
ContributorAuthor

@bnavigator I've removed sympy and lmi-sdp from the function, and I attempted to setup cvxopt in the test suite.

Copy link
Contributor

@bnavigatorbnavigator left a comment

Choose a reason for hiding this comment

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

Thanks! Some initial review comments.

@coveralls
Copy link

coveralls commentedJun 7, 2022
edited
Loading

Coverage Status

Coverage increased (+0.02%) to 94.549% when pullingc2255b0 on Mark-Yeatman:passivity-tools intod98ed29 on python-control:main.

Copy link
Member

@murrayrmmurrayrm left a comment

Choose a reason for hiding this comment

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

A few comments below. Also, it would be nice to add a unit test to make sure that if cvxopt is not installed that the proper exception is raised.

@Mark-Yeatman
Copy link
ContributorAuthor

@bnavigator I believe I've resolved all the outstanding review comments?

Copy link
Contributor

@bnavigatorbnavigator left a comment

Choose a reason for hiding this comment

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

One more review comment

@ilayn
Copy link

ilayn commentedJun 13, 2022
edited
Loading

Instead of writing things in the standard SDP notation, you might want to useCVXPY notation (which acts like YALMIP) so you don't have to use the tiresome basis matrices.

Also note that strictly proper systems cannot be checked via positive real lemma via strict inequalities due zero D + D.T block forcing the optimization problem to infeasibilty. You either have to use extended version or nudge D block withepsilon*np.eye(m). Alternatively you can solve theLMI < alpha*eye(n) and minimizealpha and if it is sufficiently small then you can call it feasible.

@Mark-Yeatman
Copy link
ContributorAuthor

Mark-Yeatman commentedJun 14, 2022
edited
Loading

I would definitely be up for using cvxpy, but it looks like it has more dependencies than just using cvxopt (https://www.cvxpy.org/install/#install-from-source vshttps://cvxopt.org/install/#building-and-installing-from-source)? That seems like an issue for a project maintainer.

You're right about the D = 0 case, I will nudge it as you suggested. Thanks for catching that.

@bnavigator
Copy link
Contributor

CVXPY: I don't think it is necessary to employ a full domain specific language with all its dependencies for one hardcoded optimization problem.

… rename is_passive to ispassive for naming convention consistency. Autoformat to pep8.
@bnavigatorbnavigator changed the titleAdd passivity module, is_passive function, and passivity_test. Introduces dependencies on lmi_sdp and cvxopt.Add passivity module, ispassive function, and passivity_test. Introduces optional dependency cvxopt.Jun 14, 2022
Mark-Yeatmanand others added4 commitsJune 14, 2022 11:26
…led in an object oriented style as a LTI class member. Added unit tests for transfer function and oo style calls. Ran autopep8 on lti.py.
@bnavigator
Copy link
Contributor

I took the liberty of fixing the missing skips. I hope you don't mind.

Copy link
Contributor

@bnavigatorbnavigator left a comment

Choose a reason for hiding this comment

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

I think this is ready for a merge. Any improvements welcome with subsequent PRs.

Mark-Yeatman reacted with hooray emoji
@murrayrmmurrayrm merged commit2f29a4c intopython-control:mainJun 16, 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

@murrayrmmurrayrmmurrayrm left review comments

@JBannwarthJBannwarthJBannwarth left review comments

@bnavigatorbnavigatorbnavigator approved these changes

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

Successfully merging this pull request may close these issues.

6 participants
@Mark-Yeatman@bnavigator@coveralls@ilayn@murrayrm@JBannwarth

[8]ページ先頭

©2009-2025 Movatter.jp