Skip to content

Improve acker pole placement doc #1006

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

Merged
merged 3 commits into from
May 23, 2024

Conversation

pierre-haessig
Copy link
Contributor

Hello,

This is a follow-up of #1004, still on pole placement, but now turning to acker docstring.

As a first step, this adds a "See Also" section to place and place varga.

However, I have two more ideas to extend this PR:

  • add reference to the Ackermann's formula. I've found § 2.3.5 in the "Linear Control Theory" book by F. W. Fairman, 1998, but I'm not sure it matches perfectly with what's implemented.
  • to the best of my (quite limited) knownledge, Ackermann's formula is for single input systems (and I got some linear algebra error on a test with a system with 2 inputs). If correct, I suggest to add a ValueError if B.shape[1] > 1.

@coveralls
Copy link

coveralls commented May 21, 2024

Coverage Status

coverage: 94.504%. remained the same
when pulling 60a7bfa on pierre-haessig:doc-acker
into 87a0550 on python-control:main.

@pierre-haessig
Copy link
Contributor Author

Oups, I didn't see PR #1007 and pushed my own commit to fix place_varga doctring. Now I have to fix the conflict!

(mostly reverts commit commit 4f0e9b7)
@pierre-haessig
Copy link
Contributor Author

Now I have to fix the conflict!

Done!

@slivingston slivingston self-assigned this May 23, 2024
@slivingston
Copy link
Member

@pierre-haessig Thanks! I am going to squash-merge this because the second commit is not used after you resolved the conflict.

@slivingston slivingston merged commit e1d21d2 into python-control:main May 23, 2024
23 checks passed
@slivingston
Copy link
Member

Regarding the other two items in your opening post:

  1. For widely known results like Ackermann's formula, I recommend to use an open access reference, if available. The book Feedback Systems can be used for this. Ackermann's formula is given in equation (7.21) in Chapter 7.
  2. Ackermann's formula is only for single-input systems. This aligns with the similar acker function in Matlab. The function always raises an exception if the system has more than 1 input, but the error message may be confusing. We could add a check with ValueError as you suggest, or the documentation can add a note about single-input systems (or both). Feel free to create another pull request with it.

As an aside, I was not able to find a description of acker in the official Matlab documentation. Perhaps they removed it?

@pierre-haessig
Copy link
Contributor Author

pierre-haessig commented May 27, 2024

As an aside, I was not able to find a description of acker in the official Matlab documentation. Perhaps they removed it?

Neither did I. I only found an online copy of a unspecified Matlab version here: www.ece.northwestern.edu/local-apps/matlabhelp/toolbox/control/ref/acker.html (in the source code, the date 2002 is mentioned). I don't know when it was removed.

[Edit: sorry, I didn't see you were mentioning the exact same link for Matlab's acker doc...]

@murrayrm murrayrm added this to the 0.10.1 milestone Jun 30, 2024
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.

4 participants