Skip to content

Commit

Permalink
BUG: use numpy.ndarray, instead of numpy.matrix
Browse files Browse the repository at this point in the history
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.


## 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 the warning 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.

Running a single test function using `pytest` is possible by writing:

```shell
pytest abstract_test.py::name_of_function
```


## 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
  • Loading branch information
johnyf committed Aug 20, 2021
1 parent d6f21b4 commit 4265d82
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 17 deletions.
2 changes: 1 addition & 1 deletion tests/prop2part_test.py
Expand Up @@ -35,7 +35,7 @@ def prop2part_test():
print(mypartition)

ref_adjacency = np.array([[1,0,1],[0,1,1],[1,1,1]])
assert np.all(mypartition.adj.todense() == ref_adjacency)
assert np.all(mypartition.adj.toarray() == ref_adjacency)

assert len(mypartition.regions) == 3

Expand Down
20 changes: 6 additions & 14 deletions tulip/abstract/discretization.py
Expand Up @@ -642,23 +642,19 @@ def _discretize_bi(
else:
rd = 0.
# Initialize matrix for pairs to check
IJ = part.adj.copy()
IJ = IJ.todense()
IJ = np.array(IJ)
IJ = part.adj.copy().toarray()
logger.debug("\n Starting IJ: \n" + str(IJ) )
# next line omitted in discretize_overlap
IJ = reachable_within(trans_length, IJ,
np.array(part.adj.todense()) )
part.adj.toarray())
# Initialize output
num_regions = len(part)
transitions = np.zeros(
[num_regions, num_regions],
dtype = int
)
sol = deepcopy(part.regions)
adj = part.adj.copy()
adj = adj.todense()
adj = np.array(adj)
adj = part.adj.copy().toarray()
# next 2 lines omitted in discretize_overlap
if ispwa:
subsys_list = list(ppp2pwa)
Expand Down Expand Up @@ -1106,23 +1102,19 @@ def _discretize_dual(
else:
rd = 0.
# Initialize matrix for pairs to check
IJ = part.adj.copy()
IJ = IJ.todense()
IJ = np.array(IJ)
IJ = part.adj.copy().toarray()
logger.debug("\n Starting IJ: \n" + str(IJ) )
# next line omitted in discretize_overlap
IJ = reachable_within(trans_length, IJ,
np.array(part.adj.todense()))
part.adj.toarray())
# Initialize output
num_regions = len(part)
transitions = np.zeros(
[num_regions, num_regions],
dtype = int
)
sol = deepcopy(part.regions)
adj = part.adj.copy()
adj = adj.todense()
adj = np.array(adj)
adj = part.adj.copy().toarray()
# next 2 lines omitted in discretize_overlap
if ispwa:
subsys_list = list(ppp2pwa)
Expand Down
4 changes: 2 additions & 2 deletions tulip/abstract/prop2partition.py
Expand Up @@ -586,9 +586,9 @@ def __str__(self):

s += str(region)

if hasattr(self.adj, 'todense'):
if hasattr(self.adj, 'toarray'):
s += 'Adjacency matrix:\n'
s += str(self.adj.todense()) + '\n'
s += str(self.adj.toarray()) + '\n'
return s

def plot(
Expand Down

0 comments on commit 4265d82

Please sign in to comment.