Repository navigation
Decide on how to handle (to be deprecated?) np.matrix class #233
Description
Activity
It would be nice to get this cleaned up. Flagging for release in 0.8.2.
FYI, I've started working on cleaning this up. It involves substantial changes to test cases and the
StateSpaceclass.Hello,
maybe I should let you know that I already performed part of this cleaning in currently opened PRs, like here:
https://github.com/python-control/python-control/pull/268/filesUpdated code posted in PR #292. One question is whether we should incorporate this in version 0.8.2 or kick up the revision number and make the version be 0.9.0 when the PR is merged. All of my testing shows that everything in PR #292 is backward compatible, so I think it is OK to leave the update at the patch level (versus minor level).
@don4get We'll have to resolve the conflicts in the two PRs at some point. I changed lots of code in the unit tests, so it will probably generate conflicts with several of your (and other PRs). Sorry about that, but didn't see any other way to proceed since it made sense to update things all at once in PR #292.
Related to this issue, I've noticed that we are often importing both Numpy and Scipy but then accessing numpy objects like
arrayandmatrixusing thespreference.For example, in control/bdalg.py:
import scipy as sp import numpy as np ... Q = sp.mat([ [ 1, 2], [2, -1] ])In many cases we don't need to import scipy at all since many of these objects are actually from numpy originally:
In [1]: import numpy as np In [2]: import scipy In [3]: scipy.array is np.array Out[3]: True In [4]: scipy.mat is np.mat Out[4]: True In [5]: scipy.zeros is np.zeros Out[5]: True In [6]: scipy.transpose is np.transpose Out[6]: TrueI haven't seen
import scipy as spbefore so not sure it's a common convention. Scipy is a big library so usually you just import specific things from one of the sub-modules, like so:from scipy.integrate import odeintUnless there is a special reason to use
import scipy as spI propose eliminating these wherever possible as part of thisnp.matrixremoval project.B.t.w. I also saw a couple of
from scipy import *which is definitely not a good idea (inserts over 600 names into the global namespace).I'm happy to have a go at this, and realise it may not be a top priority.
As a general principle, I think we should use numpy whenever possible. Definitely any references to
sp.arrayorsp.matrixshould convert to numpy.I don't think we have converged yet on a good approach to removing the
numpy.matrixdependence. PR #292 is a first attempt, but it has lots of problem. I think we should first sort out what we want to accomplish and then one of us can put together a PR addressing this issue (probably replacing #292).Some things I think we should tackle:
-
We need to decide whether we want the
controlpackage or thecontrol.matlabsubpackage to allow MATLAB-style matrix operations (*, ^, etc). I can see two arguments: (1) we should by pythonic and phase out all MATLAB-style operations or (2) controls analysis does a lot to of matrix multiplication and inversion => we should support these as fundamental operations.I think I am leaning toward just doing everything with arrays (so staying "pythonic" or perhaps "numpythonic" about things). For anyone using Python3.5 and up, you get matrix multiplication with
@and the only thing you lose is matrix powers. -
We need to decide on some guidelines for return values from
controlpackage functions. Right now we return a combinations of lists, arrays, and matrices, depending on who wrote the code. I think it would be useful to standardize things a bit, so that you don't have to constantly be looking at the documentation to figure out what type of object you are getting. -
We need to have some means of backward compatibility and decide at what point we are willing to break running code. What I think I like best of the ideas we have played with so far is the notion of including a command
use_numpy_matrix()that would set configuration variables so that you stay compatible with current practtice. We can then decide at what point we makeuse_numpy_matrix(False)the default and what we return instead. -
We should clean up our scipy/numpy usage so that we only use scipy when something isn't in numpy and we don't import entire namespaces when we don't need them.
-
I agree we need to think through what we are trying to achieve. I don't think we should be trying to replicate Matlab in Python. There are fundamental differences that we wouldn't want to replicate (such as zero-based indexing and semi-colons as line-terminators).
On the other hand, it seems a shame that Numpy is deprecating the
matrixclass as it seems like a legitimate use-case - not everyone works in n dimensions all the time and the mental overhead of keeping track of the dimension and shape of your array object is quite large.I've been doing some playing around to check out the differences between numpy ndarray and matrix classes. Then I found this more thorough compare-and-contrast article in the numpy documentation (if you haven't seen it). B.t.w. There's also a reddit discussion.
To me, the differences are few but so fundamental (e.g. what
*operator does) that we can't have ambiguity about whether an object is a matrix or an ndarray. So a transition to ndarrays with a temporary 'global switch' that turns everything into matrix objects (inputs and outputs) seems reasonable to me.I think it will mean a lot of
.reshape()methods appearing in the code though which is unfortunate and users will have to get their heads around ndarrays and the new operators.@billtubbs The reasons are similar to what you are trying to do here; trying to get rid of a thin-matlab user pleasing compatibility layer. It was historically similar but it turned out to be very difficult to keep track of "is it a matrix or array" question. Note that once it is in, it is very difficult to take something out from NumPy/SciPy codebase.
If it wasn't for the
scipy.sparsemodule it would already be deprecated however it turned out to be quite challenging to get rid of it so things are a bit stalled now. Also subclassing is another detail against matrix. Hence it is just not removing it independent from other objects in the library. Finally it is really a very limited design as a generic object for subclassing as you are experiencing now.One thing we could do is maintain a matrix-like class in the
control.matlabmodule (which attempts to provide a more MATLAB-like environment for easier transition). But I still feel like pushing users to Python 3.5+ (where@becomes available) and then just using ndarray's is the best bet.Thanks @ilayn . Yes, I saw a similar explanation on Reddit by someone from the Scipy team.
@murrayrm I was thinking about that option too. But we would in effect be trying to fix or replace np.matrix class which would be going against the grain with the rest of the community and presumably we would face all the issues currently dogging
np.matrix.Back to your original question, are we trying to make a package geared towards Matlab users requiring minimal re-learning or are we trying to make a Python control package that python users can adopt with minimal re-learning? The ndarray vs matrix question is just an example that provokes that bigger question. I think we need to go one way or the other and not hover somewhere in-between.
A proposal for how to move forward:
-
We take as our eventual goal the use of
numpy.ndarrayas the standard class for all state space objects inpython-control. This will be consistent with where the community is heading. By default, all arrays related to state space functions would be 2D objects (but see items below regarding allowable input forms). -
As a bridge to keep everyone's code from breaking, we implement a
use_numpy_matrixfunction inconfig.pythat sets the internal type of theStateSpaceclass tonumpy.matrixand output type for allpython-controlfunctions that return a matrix tonumpy.matrix. The default for the next release would beuse_numpy_matrix(True)so that nobody's code breaks, but at a future date we makeuse_numpy_matrix(False)as the default. -
For all
python-controlfunctions that take row or column vectors as inputs, we allow these to be specified either as a 0D object, 1D object, or 2D object (scalar, list, list of lists, ndarray, etc) as long as it can be converted to the right shape. So all of the following would work and give you the same result:# sys = control.StateSpace(A, B, C, D) sys = control.StateSpace([[-1, 1], [0, -1]], [1, 0], [0, 1], 0) sys = control.StateSpace([[-1, 1], [0, -1]], [[1], [0]], [0, 1], 0) sys = control.StateSpace([[-1, 1], [0, -1]], [[1], [0]], [[0, 1]], [[0]])(the last one matches the shape of what would be stored internally).
In addition, if you are using
numpy.matrixas the default (viause_numpy_matrix(True)) then you could also pass MATLAB-style strings as inputs (this is probably the biggest place where we will break things when we move away fromnumpy.matrixtonumpy.ndarray). -
For all
python-controlfunctions that return matrices as outputs (including column and row vectors), you get a 2Dndarrayof the appropriate shape. For row or column vectors, the users can usesqueezeif they want to turn the 2D object back into a 1D object. (Alternatively, we could add asqueezekeyword, as we have done inforced_response().)Note that shifting to a consistent output for all functions could "break" some current user code since current functions vary in whether they return 2D
numpy.ndarrayobjects or anumpy.matrixobjects. Now all functions would return one or the other (depending onuse_numpy_matrixconfig setting). -
For the
python.matrixsubpackage, we follow the conventions of thecontrolpackage (eventual conversion tondarray) and put information in the documentation that lets people know they can useuse_numpy_matrixif they want something more MATLAB-like (for as long as thenumpy.matrixclass is supported). -
We convert unit tests to move away from the use of
numpy.matrixobjects, though with some explicit unit tests to make surenumpy.matrixobjects are allowed for as long as they are supported. Warning messages should be turned off when usingnumpy.matrixon purpose, to get rid of extraneous warning messages.
Note that compared with PR #292, this would get rid of the
StateSpaceMatrixclass and the problems associated with that approach (described above).Thoughts?
-
I think the ultimate goal is the right one and the transition approach makes sense to me unless someone comes up with a better method.
I don't think we can ever overcome the fact that Python is a general-purpose language and so it will never be as perfect as Matlab for control tasks. I.m.h.o. our goal is to give the Python community a good control toolbox that makes sense to Python programmers not try to replicate Matlab language features.
Implementation Idea
I was wondering if we could make use of function decorators during this transition. I.e. design a function decorator to handle all the conversion of arguments and return values according to the
use_numpy_matrixsetting. This would be nice because then we could focus all our work on making the codenp.arraycompliant and then just remove all the decorators one day if/when we no longer supportnp.matrix.I'm not an expert on decorators so this could probably be written better but it seems to work:
# Problem: function expects np.arrays but arguments could be np.matrix def some_func(A, B): return A*B # Works with np.arrays A = np.array([[1, 2], [3, 4]]) B = np.array([[1, 0], [0, 1]]) result1 = some_func(A, B) print(result1, type(result1)) # But result is different with np.matrix A = np.matrix([[1, 2], [3, 4]]) B = np.matrix([[1, 0], [0, 1]]) result2 = some_func(A, B) print(result2, type(result2)) (result1 == result2).all()Output:
[[1 0] [0 4]] <class 'numpy.ndarray'> [[1 2] [3 4]] <class 'numpy.matrix'> False# Solution: define a decorator import functools def convert_arguments(func): @functools.wraps(func) def wrapper_decorator(*args, **kwargs): # Convert arguments converted_args = [] for arg in args: if isinstance(arg, np.matrix): print("Matrix arg converted") arg = np.array(arg) converted_args.append(arg) args = tuple(converted_args) # Execute function value = func(*args, **kwargs) # Convert return values if use_numpy_matrix: value = np.matrix(value) print("Matrix return value converted") return value return wrapper_decorator # Then use it on every function: @convert_arguments def some_func(A, B): """Some function that expects np.arrays.""" return A*B # Test with np.arrays use_numpy_matrix = False A = np.array([[1, 2], [3, 4]]) B = np.array([[1, 0], [0, 1]]) result1 = some_func(A, B) print(result1, type(result1)) # Test with np.matrix use_numpy_matrix = True A = np.matrix([[1, 2], [3, 4]]) B = np.matrix([[1, 0], [0, 1]]) result2 = some_func(A, B) print(result2, type(result2)) (result1 == result2).all()Output:
[[1 0] [0 4]] <class 'numpy.ndarray'> Matrix arg converted Matrix arg converted Matrix return value converted [[1 0] [0 4]] <class 'numpy.matrix'>A proposal for how to move forward:
* We take as our eventual goal the use of `numpy.ndarray` as the standard class for all state space objects in `python-control`. This will be consistent with where the community is heading. By default, all arrays related to state space functions would be 2D objects (but see items below regarding allowable input forms).:
:
Thoughts?I think the overall goal of this proposal is good. In general, I'm of the opinion that it's better to follow the conventions of the scientific python community than to try and emulate matlab, and I think this proposal moves us in that direction.
What's the latest on this? I might have some more time in coming weeks but I would need to be assigned some tasks that are within my skills and abilities.
B.t.w. I came across this project which was written to replace the original PILCO algorithm implementation written in MATLAB. Maybe we can learn something from the approach? I notice they do use the
@operator wherever possible but they are also using the matrix methods in TensorFlow a lot. Example:# Calculate S: Predictive Covariance R = s @ tf.matrix_diag( 1/tf.square(self.lengthscales[None, :, :]) + 1/tf.square(self.lengthscales[:, None, :]) ) + tf.eye(self.num_dims, dtype=float_type)They are also using decorators for converting input arguments it seems:
@params_as_tensors def compute_reward(self, m, s): ...Ah sorry, I wasn't paying attention. So it's all done then?
Let me know if there is something else I can help with.Yes, effectively done. When we decide we want to get rid of the matrix class, we just change the default setting in
config.py.There are lots of other places to pitch in (43 other open issues... -:).
The latest version of
numpyis listing thematrixclass as something that may be deprecated in the future (see numpy-discussion thread). This is currently generating warning messages in our unit tests (see issue #232) but otherwise not causing any harm.At some point we have to decide whether to switch out internal operations to use the
arrayclass, as recommended in the numpy documentation.