Complex matrices #376
Complex matrices #376
Conversation
control/tests/statesp_test.py
Outdated
| def test_evalfr_complex(self): | ||
| """Evaluate the frequency response at one frequency.""" | ||
|
|
||
| resp = [[4.6799374736968655 -34.9854626345217383j, |
There was a problem hiding this comment.
Please be easier on the precision of your reference values. This will almost certainly fail on some build systems due to rounding errors or depending on the used BLAS/LAPACK implementation.
|
https://travis-ci.org/python-control/python-control/jobs/656662515#L1598-L1631 |
|
What happens if you call a function that uses slycot and give it a |
|
I've run Regarding the second warning, there is a complex routine |
|
I've made a more detailed traceback of the warnings: Tests failing with
|
If we compare sys.A against A we get a IndexError (not sure why), but building a new auxiliary StateSpace seems to do the trick
If we compare sys.A against A we get a IndexError (not sure why), but building a new auxiliary StateSpace seems to do the trick
|
Well, it seems that the current f2py interface doesn't support complex arrays as an argument. I've tried to use the complex routine |
|
So you wrote a wrapper for |
|
According to the f2py typespec documentation, f2py signature files understand |
|
Yes f2py does support I'll do the pull request in a minute |
Will the CI check a new function that it is just added or there is some config file to declare with functions will be teseted? |
|
You need to write unit tests associated with any new functions that you add, otherwise they won't be checked. If you are adding something to |
|
I've changed The tests affected by this change should be |
|
The PR here in python-control will only be able to access the new function in Slycot when your PR over there is merged, a new version is released and the CI in python-control/master is updated to use this new release. So please first amend python-control/Slycot#96 with a unit test, convince the maintainers to merge and wait for a release. The python-control unit tests which are run within Slycot's CI are also from master. Of course you could also modify the CI scripts here and over at Slycot#96 to use your respective modified branches for development, but that won't be suitable for merging. Thus I recommend to develop first in Slycot and as soon as that is done continue in python-control. |
|
| arr = np.matrix(data, dtype=float) | ||
| if np.isrealobj(data): | ||
| arr = np.matrix(data, dtype=float) | ||
| elif np.iscomplexobj(data): | ||
| arr = np.matrix(data, dtype=complex) | ||
| else: | ||
| arr = np.array(data, dtype=float) | ||
| if np.isrealobj(data): | ||
| arr = np.array(data, dtype=float) | ||
| elif np.iscomplexobj(data): | ||
| arr = np.array(data, dtype=complex) |
There was a problem hiding this comment.
Is this really necessary? Why not just use matrix() and asarray() without dtype and figure out the dtype later?
There was a problem hiding this comment.
Turns out if dtype is omitted completely, it could still be int and trip operations later on.
The test suite already has complex casting warnings from Scipy and Slycot functions returning complex arrays (with 0 imaginary part), which are passed to _ssmatrix. Thus, my approach in #438 is to use dtype=np.result_type(np.float, arr), which ensures to cast from int to at least float and retains complex if arr is already complex.
It is not necessary to enforce float and can raise ComplexWarning during casting; cf. python-control#376
Now I've written new tests that use complex matrices without touching the old ones