GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Improve `acker` pole placement doc by pierre-haessig · Pull Request #1006 · python-control/python-control

Conversation

@pierre-haessig

@pierre-haessig pierre-haessig commented May 21, 2024

Copy link
Copy Markdown
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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@pierre-haessig
add "See Also" to acker's doc
3a06aed
@pierre-haessig pierre-haessig mentioned this pull request May 21, 2024
add place_varga in API doc #1004
Merged
@coveralls

coveralls commented May 21, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

Coverage Status

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@pierre-haessig
fix place_varga docstring
4f0e9b7
@pierre-haessig

pierre-haessig commented May 23, 2024

Copy link
Copy Markdown
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!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@pierre-haessig
Merge 'upstream/main' into doc-acker
60a7bfa
(mostly reverts commit commit 4f0e9b7)
@pierre-haessig

pierre-haessig commented May 23, 2024

Copy link
Copy Markdown
Contributor Author

Now I have to fix the conflict!

Done!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@slivingston slivingston self-assigned this May 23, 2024
@slivingston

slivingston commented May 23, 2024

Copy link
Copy Markdown
Member

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@slivingston
slivingston merged commit e1d21d2 into python-control:main May 23, 2024
@slivingston

slivingston commented May 24, 2024

Copy link
Copy Markdown
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?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@pierre-haessig

pierre-haessig commented May 27, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

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...]

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@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

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@slivingston slivingston

Labels

None yet

Projects

None yet

Milestone

0.10.1

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@pierre-haessig @coveralls @slivingston @murrayrm
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.