8000 BUG: use `numpy.ndarray`, instead of `numpy.matrix` · tulip-control/tulip-control@7da5534 · GitHub
[go: up one dir, main page]

Skip to content

Commit 7da5534

Browse files
committed
BUG: use numpy.ndarray, instead of numpy.matrix
because matrices have been deprecated in `numpy`. Usage of `scipy.sparse` is causing this issue when conversions to matrices are performed. I changed the: - calls to the method `scipy.sparse.lil.lil_matrix.todense` - to calls to the method `scipy.sparse.lil.lil_matrix.toarray`, to avoid the conversions that raise `PendingDeprecationWarning`s. The warnings are raised by the call to the method `lil_matrix.todense` because this call involves the instantiation of the class `numpy.matrix`, which is deprecated. In contrast, the method `lil_matrix.toarray` creates an instance of `numpy.ndarray`. (In fact, in the module `tulip.abstract.discretization`, wherever the method `lil_matrix.todense` was called, the value that it returned was immediately converted to a `numpy.ndarray`. So calling the method `lil_matrix.toarray` is actually more efficient.) The class `numpy.matrix` is deprecated and will probably be removed in the future. This will happen after arrangements have been made for `scipy.space`. (For these points and more information, read the references listed at the end.) Still, I do not think that continuing to use `scipy.sparse.lil_matrix` until when `numpy` removes matrices is a safe approach. Instead, using `numpy.ndarray` would be safer. Moreover, I do think that there are other data structures that would fit this use case better than sparse matrices. ## Diagnosis I describe below the approach I (eventually) followed to debug this warning, because finding the cause was difficult. The issue is a `PendingDeprecationWarning` issued from `numpy`. This warning is visible in `pytest` runs, but *not* when running the Python test file directly. Moreover, `pytest` reports the warning, and from which test function it originates. The warning itself reads (I have wrapped the lines here): ``` ===================================== warnings summary ====================================== abstract_test.py::transition_directions_test /.../.virtualenvs/.../lib/python3.9/site-packages/numpy/matrixlib/defmatrix.py:69: PendingDeprecationWarning: the matrix subclass is not the recommended way to represent matrices or deal with linear algebra (see https://docs.scipy.org/doc/numpy/user/numpy-for-matlab-users.html). Please adjust your code to use regular ndarray. return matrix(data, dtype=dtype, copy=False) ``` So the line in `tulip` that causes the warning cannot be found from the information contained in the warning. The above `PendingDeprecationWarning` was introduced in `numpy` in commit: numpy/numpy@11e9d2a This warning was then ignored in the module `scipy.sparse.__init__`, in `scipy` commit: scipy/scipy@a874bd5 It appears that this configuration of warnings by `scipy` interacts with `pytest` complexly: - when running with `pytest abstract_test.py`, the `PendingDeprecationWarning` is visible, but - when running with `python -X dev -- abstract_test.py`, the `PendingDeprecationWarning` is invisible. This behavior is due to the call: ```python warnings.filterwarnings( 'ignore', message='the matrix subclass is not the recommended way') ``` within `scipy.sparse.__init__.py` (introduced in the `scipy` commit that was mentioned above). Read also: https://docs.python.org/3/library/exceptions.html#PendingDeprecationWarning https://www.python.org/dev/peps/pep-0565/ As a result, it is difficult to find the cause within `tulip` of this `PendingDeprecationWarning`. ## Getting a traceback The test function that triggered the `PendingDeprecationWarning` from `numpy` was not failing, so there was no traceback that would indicate which line in `tulip` caused the warning. In addition, there was an earlier warning issued by `matplotlib`. So turning warnings to errors with the argument `-Werror` would cause `pytest` to turn the `matplotlib` warning into an error, and stop before the warning of interest: ```shell pytest -Werror abstract_test.py ``` So first I removed the `matplotlib` warnings (temporarily), by commenting the line `matplotlib.use('Agg')` in the file `abstract_test.py`. This made the warning of interest to become the first warning. I then passed `-Werror` to `pytest`, and this turned the `numpy` warning into an error, which produced the traceback shown below: (The cause of these `matplotlib` warnings (there are two) is in the package `polytope`, and has been addressed there, in commit: tulip-control/polytope@c464818 These changes will become available to `tulip` with the next `polytope` release. Until then, the CI tests of `tulip` will raise these `matplotlib` warnings. These warnings could be explicitly ignored by using `with pytest.warns`.) (The paths to `tulip` in the traceback below lead to the repository's `tulip`, instead of a directory under Python's `site-packages`, because during this phase of debugging I installed `tulip` with `pip install -e .`, to iterate faster while debugging.) ``` ../tulip/abstract/discretization.py:1666: in discretize_switched plot_mode_partitions(merged_abstr, show_ts, only_adjacent) ../tulip/abstract/discretization.py:1673: in plot_mode_partitions axs = swab.plot(show_ts, only_adjacent) ../tulip/abstract/discretization.py:187: in plot ax = ab.plot(show_ts, only_adjacent, color_seed) ../tulip/abstract/discretization.py:403: in plot ax = _plot_abstraction(self, show_ts, only_adjacent, ../tulip/abstract/discretization.py:446: in _plot_abstraction ax = ab.ppp.plot( ../tulip/abstract/prop2partition.py:600: in plot return plot_partition( .../.virtualenvs/.../lib/python3.9/site-packages/polytope/plot.py:90: in plot_partition trans = nx.to_numpy_matrix(trans, nodelist=ppp2trans) .../.virtualenvs/.../lib/python3.9/site-packages/networkx/convert_matrix.py:553: in to_numpy_matrix M = np.asmatrix(A, dtype=dtype) .../.virtualenvs/.../lib/python3.9/site-packages/numpy/matrixlib/defmatrix.py:69: in asmatrix return matrix(data, dtype=dtype, copy=False) _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ subtype = <class 'numpy.matrix'> data = array([[1., 0., 0., 0., 0., 0.], [0., 1., 1., 0., 1., 1.], [1., 0., 1., 1., 0., 1.], [1., 0., 0., 1., 0., 0.], [0., 0., 0., 0., 1., 1.], [1., 0., 0., 0., 0., 1.]]) dtype = None, copy = False def __new__(subtype, data, dtype=None, copy=True): > warnings.warn('the matrix subclass is not the recommended way to ' 'represent matrices or deal with linear algebra (see ' 'https://docs.scipy.org/doc/numpy/user/' 'numpy-for-matlab-users.html). ' 'Please adjust your code to use regular ndarray.', PendingDeprecationWarning, stacklevel=2) E PendingDeprecationWarning: the matrix subclass is not the recommended way to represent matrices or deal with linear algebra (see https://docs.scipy.org/doc/numpy/user/numpy-for-matlab-users.html). Please adjust your code to use regular ndarray. .../.virtualenvs/.../lib/python3.9/site-packages/numpy/matrixlib/defmatrix.py:116: PendingDeprecationWarning ``` As the traceback shows, the issue is due to a call to the function `networkx.to_numpy_matrix` within the function `polytope.plot.plot_partition`. So avoiding this warning will be possible after the next release of the package `polytope`. (Note that inserting an `assert False` in a suitable line within the function `transition_directions_test` is not an alternative to passing the argument `-Werror`, because the `assert False` will result in a traceback where the `assert` statement appears, instead of a traceback that shows the call stack at the point where the warning was issued.) ## Speeding up debugging using `pytest` Also, since I had to be running `pytest` on the Python file `abstract_test.py`, `pytest` would collect all test functions, and run them. The file `abstract_test.py` happens to contain several slow test functions, so running them all just to observe the results for the one function of interest is not time-efficient. What I did to speed up runs was to rename all `test_*` functions contained in `abstract_test.py`, except for the one function of interest (namely `transition_directions_test`), to identifiers outside the patterns collected by `pytest`. A simpler alternative, for use with larger test files, is to do the opposite: rename only the function of interest to a different pattern, and then change the line `python_functions = ` in the configuration file `pytest.ini`. ## References - numpy/numpy#10142 (DEP: Pending deprecation warning for matrix) - numpy/numpy#10973 (DOC: advise against use of matrix) - scipy/scipy#8887 (MAINT: filter out np.matrix PendingDeprecationWarning's in numpy >=1.15) - scipy/scipy#9734 (PendingDeprecationWarning for np.matrix with pytest) - scikit-learn/scikit-learn#12327 (PendingDeprecationWarning: the matrix subclass is not the recommended way to represent matrices) - scikit-learn/scikit-learn#13076 ([MRG] Ignore PendingDepWarnings of matrix subclass with pytest) - cvxpy/cvxpy#567 (NumPy matrix class is pending deprecation and issuing warnings) - cvxpy/cvxpy#637 (RF: Use a 2D np array instead of matrix to represent scalars) - cvxpy/cvxpy#638 (RF: Change np.matrix to np.array in several places) - cvxpy/cvxpy#644 (PendingDeprecationWarning: the matrix subclass is not the recommended way to represent matrices or deal with linear algebra) - https://docs.pytest.org/en/6.2.x/warnings.html#deprecationwarning-and-pendingdeprecationwarning
1 parent cc59695 commit 7da5534

File tree

3 files changed

+9
-17
lines changed

3 files changed

+9
-17
lines changed

tests/prop2part_test.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ def prop2part_test():
3535
print(mypartition)
3636

3737
ref_adjacency = np.array([[1,0,1],[0,1,1],[1,1,1]])
38-
assert np.all(mypartition.adj.todense() == ref_adjacency)
38+
assert np.all(mypartition.adj.toarray() == ref_adjacency)
3939

4040
assert len(mypartition.regions) == 3
4141

tulip/abstract/discretization.py

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -642,23 +642,19 @@ def _discretize_bi(
642642
else:
643643
rd = 0.
644644
# Initialize matrix for pairs to check
645-
IJ = part.adj.copy()
646-
IJ = IJ.todense()
647-
IJ = np.array(IJ)
645+
IJ = part.adj.copy().toarray()
648646
logger.debug("\n Starting IJ: \n" + str(IJ) )
649647
# next line omitted in discretize_overlap
650648
IJ = reachable_within(trans_length, IJ,
651-
np.array(part.adj.todense()) )
649+
part.adj.toarray())
652650
# Initialize output
653651
num_regions = len(part)
654652
transitions = np.zeros(
655653
[num_regions, num_regions],
656654
dtype = int
657655
)
658656
sol = deepcopy(part.regions)
659-
adj = part.adj.copy()
660-
adj = adj.todense()
661-
adj = np.array(adj)
657+
adj = part.adj.copy().toarray()
662658
# next 2 lines omitted in discretize_overlap
663659
if ispwa:
664660
subsys_list = list(ppp2pwa)
@@ -1106,23 +1102,19 @@ def _discretize_dual(
11061102
else:
11071103
rd = 0.
11081104
# Initialize matrix for pairs to check
1109-
IJ = part.adj.copy()
1110-
IJ = IJ.todense()
1111-
IJ = np.array(IJ)
1105+
IJ = part.adj.copy().toarray()
11121106
logger.debug("\n Starting IJ: \n" + str(IJ) )
11131107
# next line omitted in discretize_overlap
< F6B3 /td>
11141108
IJ = reachable_within(trans_length, IJ,
1115-
np.array(part.adj.todense()))
1109+
part.adj.toarray())
11161110
# Initialize output
11171111
num_regions = len(part)
11181112
transitions = np.zeros(
11191113
[num_regions, num_regions],
11201114
dtype = int
11211115
)
11221116
sol = deepcopy(part.regions)
1123-
adj = part.adj.copy()
1124-
adj = adj.todense()
1125-
adj = np.array(adj)
1117+
adj = part.adj.copy().toarray()
11261118
# next 2 lines omitted in discretize_overlap
11271119
if ispwa:
11281120
subsys_list = list(ppp2pwa)

tulip/abstract/prop2partition.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -586,9 +586,9 @@ def __str__(self):
586586

587587
s += str(region)
588588

589-
if hasattr(self.adj, 'todense'):
589+
if hasattr(self.adj, 'toarray'):
590590
s += 'Adjacency matrix:\n'
591-
s += str(self.adj.todense()) + '\n'
591+
s += str(self.adj.toarray()) + '\n'
592592
return s
593593

594594
def plot(

0 commit comments

Comments
 (0)
0