-
Notifications
You must be signed in to change notification settings - Fork 439
First commit of balred2() added to modelsimp.py. #78
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
Conversation
Looks promising. I haven't looked at this closely, but I have a couple of remarks at first glance:
|
The test failure is the same thing as reported in #70; exactly the same gain margins were found by @whiplash01 as Travis produced for Python 2.7 and 3.4 here. I can't reproduce this on my machine (Ubuntu 14.04, Python 3.4.3) with "own-built" Slycot and python-control. I have struggled to reproduce the Travis environment with my relatively slow network connection; Conda seems to want to download an awful lot of things, and then change it's mind about Python versions, etc. Anyway, it looks like there's a subtle bug lurking in the gain margin code. |
…ystems to balred.
I have modified balred() to now just check for whether or not unstable eigenvalues exist, and if so, perform the necessary steps to do the balancing and truncating. |
My first two commits were failing on testBalred(), when orders was passed as a vector(list). I've fixed this error and now the balred() function will return a vector(list) of reduced order models if orders is passed as a vector. I don't know what is going on with the testCombi01(). |
Travis build is failing due to older version of conda (fixed in f334b1d). This will be corrected when code is merged into master. |
Could you please explain what unit tests are or direct me to a reference. |
A good introduction is here: https://en.wikipedia.org/wiki/Unit_testing If you look in control/tests, you'll see a large collection of unit tests for different functions. If you copy one of those into something called |
Since the modified routine is in Also, if you rebase your branch to the current master branch, then the tests will be run automatically by Travis CI when you push more commits to this pull request. The current version in this PR has a bug in setting things up on Travis CI, which leads to the error showing up here. (By the way, @murrayrm, it might be good at some point to organize the existing tests better, to follow the NumPy/SciPy guidelines, for instance so that tests for module |
…unstable systems along with 'matchdc' gain option. Also added option to gram() to return cholesky factored gramian.
@mdclemen, will you consider opening a new pull request or reworking commits on this one (and re-opening it)? I can work with you to make unit tests. |
@slivingston, I have made updates to my fork of python-control and it requires the updates I made to slycot. I will wait to get my pull for python-control/slycot merged first. |
This pull request contains a function, balred2(), which will do a balanced, truncated realization of an unstable LTI model.