Repository navigation
LQR check inputs - #274
Closed
don4get wants to merge 12 commits into
Closed
LQR check inputs#274don4get wants to merge 12 commits into
don4get wants to merge 12 commits into
Conversation
- Fix pep8 warnings - Clean some comments - Add module docstring
- Make place_varga protected -> _place_varga - Make acker protected -> _acker - place: Add optional arguments (alpha, dtime, method) - Add fallback strategy in case scipy or slycot are not found - Update docstrings - Update tests and remove pass condition in case which "slycot is not installed." - testPlace do not expect anymore a ValueError in case redundant place location is found. Instead, the algorithm fall back on the Ackermann method while printing a warning. TODO: Complete pole placement example when PR#267 is merged.
don4get
force-pushed
the
feature/lqr-check-inputs
branch
from
April 13, 2019 12:44
5c3e31e to
2afed81
Compare
Member
|
Hard to tell what this PR does since it builds on top of #268. Is it possible to make this PR independent of that one, so we can see what is going on? Additional suggestions:
Alternatively, we can put this PR on hold until the status of PR #268 is resolved and proceed from there. |
Member
|
PR has been idle for > 6 months => stale. @don4get I will close this PR in a few days unless you will have time to work on it soon. No worries if not; just re-open when you have responded to the comments above. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
As asked in issue #252, I added a check in LQR for the positiveness of the matrix [Q N; N' R].
So I enabled again the previous unit test and wait for a
ValueErrorto be raised.I also did a little bit of pep8 cleaning.
This PR needs to be merged after this one: #268