[WIP] Improve matrix slice

[sylee957]

References to other Issues or PRs

Brief description of what is fixed or changed

I have added doit for the following features

1. Identity of slicing
   

2. Block matrix slicing if no blocks are broken
   

3. Zero matrix, one matrix will be resized to an another zero matrix and one matrix
   

4. Slicing matrix symbol into an another matrix symbol with different dimensions
   

5. Slicing a permutation matrix into an another permutation matrix (Similar to slicing a block diagonal matrix into a block diagonal matrix)
   

Things that are work in progress are

1. Slicing a FuncMatrix into an another FuncMatrix

   • The obvious thing that can be implemented is, if the slicing begins from zero and if the step is one, it can only reduce the dimension of the FuncMatrix
   • The complicated stuff is, the function can also be modified with some shifted and scaled indices. Even if this transformation is possible, I'm not sure that the result can obviously be simpler than the original result, so will find a place to put this in.

2. Slicing a block matrix while breaking some blocks.

   • I'm thinking of implementing this as a feature in block_collapse rather than the doit.

Also, I've added a rewriting method for MatrixSlice to MatrixPermute, and MatrixPermute to MatrixSlice if the permutation is simply a reversing permutation.

I have only implemented for the cases if the slicing specification are integers. But this can be improved later if there are some symbolic cases that can be logically reasonably sliced.

Other comments

This may have to be tested to cover up every cases, And I would try to implement some rewriting schemes from the MatrixPermute.

Release Notes

• matrices
  • Made error messages more descriptive for MatrixSlice.
  • Added doit for MatrixSlice for canonicalization.

[sympy-bot]

✅

Hi, I am the SymPy bot (v160). I'm here to help you write a release notes entry. Please read the guide on how to write release notes.

Your release notes are in good order.

Here is what the release notes will look like:

• matrices

  • Made error messages more descriptive for MatrixSlice. (#18298 by @sylee957)

  • Added doit for MatrixSlice for canonicalization. (#18298 by @sylee957)

This will be added to https://github.com/sympy/sympy/wiki/Release-Notes-for-1.7.

Note: This comment will be updated with the latest check if you edit the pull request. You need to reload the page to see it.

Click here to see the pull request description that was parsed.

<!-- Your title above should be a short description of what
was changed. Do not include the issue number in the title. -->

#### References to other Issues or PRs
<!-- If this pull request fixes an issue, write "Fixes #NNNN" in that exact
format, e.g. "Fixes #1234". See
https://github.com/blog/1506-closing-issues-via-pull-requests . Please also
write a comment on that issue linking back to this pull request once it is
open. -->


#### Brief description of what is fixed or changed

I have added `doit` for the following features

1. Identity of slicing 
 ![image](https://user-images.githubusercontent.com/34944973/72315501-55497480-36d6-11ea-80ec-362cc12fabab.png)

2. Block matrix slicing if no blocks are broken
![image](https://user-images.githubusercontent.com/34944973/72315914-c6d5f280-36d7-11ea-8f02-5f84928e3b08.png)

3. Zero matrix, one matrix will be resized to an another zero matrix and one matrix
 ![image](https://user-images.githubusercontent.com/34944973/72316031-28965c80-36d8-11ea-860c-91a3a315813c.png)

4. Slicing matrix symbol into an another matrix symbol with different dimensions
 ![image](https://user-images.githubusercontent.com/34944973/72316166-90e53e00-36d8-11ea-8580-4617c8a015a2.png)

5. Slicing a permutation matrix into an another permutation matrix (Similar to slicing a block diagonal matrix into a block diagonal matrix)
![image](https://user-images.githubusercontent.com/34944973/72316282-005b2d80-36d9-11ea-913c-60b27af3ebc1.png)

Things that are work in progress are
1. Slicing a `FuncMatrix` into an another `FuncMatrix`
   - The obvious thing that can be implemented is, if the slicing begins from zero and if the step is one, it can only reduce the dimension of the `FuncMatrix`
   - The complicated stuff is, the function can also be modified with some shifted and scaled indices. Even if this transformation is possible, I'm not sure that the result can obviously be simpler than the original result, so will find a place to put this in.

2. Slicing a block matrix while breaking some blocks.
   - I'm thinking of implementing this as a feature in `block_collapse` rather than the `doit`.

Also, I've added a rewriting method for `MatrixSlice` to `MatrixPermute`, and `MatrixPermute` to `MatrixSlice` if the permutation is simply a reversing permutation.

I have only implemented for the cases if the slicing specification are integers. But this can be improved later if there are some symbolic cases that can be logically reasonably sliced. 

#### Other comments

This may have to be tested to cover up every cases, And I would try to implement some rewriting schemes from the `MatrixPermute`.

![Screen Shot 2020-01-25 at 19 51 29](https://user-images.githubusercontent.com/34944973/73119987-4a32f600-3fac-11ea-9440-30b51f4a23d4.png)


#### Release Notes

<!-- Write the release notes for this release below. See
https://github.com/sympy/sympy/wiki/Writing-Release-Notes for more information
on how to write release notes. The bot will check your release notes
automatically to see if they are formatted correctly. -->

<!-- BEGIN RELEASE NOTES -->
- matrices
  - Made error messages more descriptive for `MatrixSlice`.
  - Added `doit` for `MatrixSlice` for canonicalization.
<!-- END RELEASE NOTES -->

[oscarbenjamin]

Is this still WIP? Does it fix #18411?

[sylee957]

I didn’t make up the patch for the issue yet.
I’m reviewing things like how the slicing is handled for Range, Array, so it may take some time

[codecov]

Codecov Report

Merging #18298 into master will increase coverage by 0.047%.
The diff coverage is 72.772%.

@@              Coverage Diff              @@
##            master    #18298       +/-   ##
=============================================
+ Coverage   75.285%   75.333%   +0.047%     
=============================================
  Files          635       639        +4     
  Lines       167071    167285      +214     
  Branches     39423     39465       +42     
=============================================
+ Hits        125781    126021      +240     
+ Misses       35748     35705       -43     
- Partials      5542      5559       +17     

[sylee957]

One thing that makes it difficult to comprehend python slice is that, for some cases, None cannot be normalized to any integer.
e.g.

>>> A = list(range(10))
>>> A[slice(None, None, -1)]
[9, 8, 7, 6, 5, 4, 3, 2, 1, 0]
>>> A[slice(None, None, -2)]
[9, 7, 5, 3, 1]
>>> A[slice(5, None, -1)]
[5, 4, 3, 2, 1, 0]
>>> A[slice(5, None, -2)]
[5, 3, 1]

I think that it only applies for some case like stop is None and step is negative.
I'm thinking of adding a new singleton symbol like SliceNone or BooleanNone to sympy.

[sylee957]

Or an alternative way can be redefining the slicing notation to begin exactly from the start and stop exactly at the end for internal comprehension, unlike following the same indexing used in python. (And print back to pythonic notation only for displays)

And we may be able to comprehend those None into some meaningful integers.

I have decided to follow up the above definition

[czgdp1807]

Any updates on this?

[jlherren]

I accidentally started doing similar changes before I noticed this pull request :(

It would be nice if this would also work for symbolic slices, for example:

n = symbols('n', integer=True)
A, B, C, D = [MatrixSymbol(x, n, n) for x in 'ABCD']
K = BlockMatrix([[A, B], [C, D]])
print(block_collapse(K[:n, :n]))

Further, is there a reason why OneMatrix(8, 8)[:2, :2] should not evalute immediately? My understanding is that class constructors do not evaluate by default, but operators do.

And finally, wouldn't it be better to call self._eval_slice() in MatrixExpr and then have each class deal with their own slicing, instead of having all those if isinstance(parent, ...) in doit()?

[czgdp1807]

@sylee957 Any thoughts on #18298 (comment)