Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add optimization of numpy code involving matrix inverses #17041

Merged
merged 8 commits into from Jun 24, 2019

Conversation

Projects
None yet
5 participants
@anpandey
Copy link
Contributor

commented Jun 17, 2019

Brief description of what is fixed or changed

Adds an optimization option to the code printer base class that executes any codegen.rewriting optimizations the printer object might have. Also adds an optimization to the numpy printer that uses the assumption system to rewrite the multiplication of a matrix and a vector as the faster np.linalg.solve when appropriate.

For example:

>>> from sympy.assumptions import assuming, Q
>>> from sympy.matrices import MatrixSymbol
>>> from sympy.printing.pycode import NumPyPrinter
>>> A = MatrixSymbol('A', 2, 2)
>>> x = MatrixSymbol('x', 2, 1)
>>> p = NumPyPrinter(settings={'optimize' : True})
>>> print(p.doprint(A**(-1) * x))
(numpy.linalg.inv(A)).dot(x)
>>> with assuming(Q.fullrank(A)):
...     p.doprint(A**(-1) * x)
numpy.linalg.solve(A, x)

Other comments

This PR is mostly meant as an exploration of how (and whether) the codegen.rewriting module is enough for generating optimizations. I'd appreciate comments and suggestions about this approach.

Release Notes

  • printing
    • Add support for optimizations to numpy printer.
    • The numpy and octave printers print matrix expressions like A**-1*x using a solve.
Add optimization of numpy code involving matrix inverses
Adds an `optimization` option to the code printer base class that
executes any `codegen.rewriting` optimizations the printer object might
have. Also adds an optimization to the numpy printer that uses the
assumption system to rewrite the multiplication of a matrix and a vector
as the faster `np.linalg.solve` when appropriate.

For example:

    >>> from sympy.assumptions import assuming, Q
    >>> from sympy.matrices import MatrixSymbol
    >>> from sympy.printing.pycode import NumPyPrinter
    >>> A = MatrixSymbol('A', 2, 2)
    >>> x = MatrixSymbol('x', 2, 1)
    >>> p = NumPyPrinter(settings={'optimize' : True})
    >>> print(p.doprint(A**(-1) * x))
    (numpy.linalg.inv(A)).dot(x)
    >>> with assuming(Q.fullrank(A)):
    ...     p.doprint(A**(-1) * x)
    numpy.linalg.solve(A, x)
@sympy-bot

This comment has been minimized.

Copy link

commented Jun 17, 2019

Hi, I am the SymPy bot (v147). 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:

  • printing
    • Add support for optimizations to numpy printer. (#17041 by @anpandey)

    • The numpy and octave printers print matrix expressions like A**-1*x using a solve. (#17041 by @anpandey)

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

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. -->

#### Brief description of what is fixed or changed
Adds an `optimization` option to the code printer base class that executes any `codegen.rewriting` optimizations the printer object might have. Also adds an optimization to the numpy printer that uses the assumption system to rewrite the multiplication of a matrix and a vector as the faster `np.linalg.solve` when appropriate.

For example:

```python3
>>> from sympy.assumptions import assuming, Q
>>> from sympy.matrices import MatrixSymbol
>>> from sympy.printing.pycode import NumPyPrinter
>>> A = MatrixSymbol('A', 2, 2)
>>> x = MatrixSymbol('x', 2, 1)
>>> p = NumPyPrinter(settings={'optimize' : True})
>>> print(p.doprint(A**(-1) * x))
(numpy.linalg.inv(A)).dot(x)
>>> with assuming(Q.fullrank(A)):
...     p.doprint(A**(-1) * x)
numpy.linalg.solve(A, x)
```

#### Other comments

This PR is mostly meant as an exploration of how (and whether) the `codegen.rewriting` module is enough for generating optimizations. I'd appreciate comments and suggestions about this approach.

#### Release Notes


<!-- BEGIN RELEASE NOTES -->
* printing
  * Add support for optimizations to numpy printer.
  * The numpy and octave printers print matrix expressions like `A**-1*x` using a `solve`.
<!-- END RELEASE NOTES -->

Update

The release notes on the wiki have been updated.

@codecov

This comment has been minimized.

Copy link

commented Jun 17, 2019

Codecov Report

Merging #17041 into master will increase coverage by 0.032%.
The diff coverage is 93.103%.

@@              Coverage Diff              @@
##            master    #17041       +/-   ##
=============================================
+ Coverage   74.435%   74.467%   +0.032%     
=============================================
  Files          622       623        +1     
  Lines       161085    161114       +29     
  Branches     37811     37814        +3     
=============================================
+ Hits        119904    119978       +74     
+ Misses       35861     35815       -46     
- Partials      5320      5321        +1
@asmeurer

This comment has been minimized.

Copy link
Member

commented Jun 17, 2019

Last I saw Travis didn't run on draft pull requests, but maybe that has been fixed because it looks like it ran here. Anyway, if you have any issues where Travis doesn't run, you may need to make the PR not a draft (just use WIP in the title instead).

Show resolved Hide resolved sympy/printing/pycode.py Outdated
@asmeurer

This comment has been minimized.

Copy link
Member

commented Jun 17, 2019

@bjodah any thoughts on this approach?

@bjodah

This comment has been minimized.

Copy link
Member

commented Jun 17, 2019

Definitely an interesting approach. I only had time to glance over it at this time. Are the optimizations exclusive to numpy? Wouldn't the octave printer benefit from similar rewrites (I don't know if it applies to R/julia/mathematica too)? In that case the predicate/transformers, which can be shared, could go somewhere else under sympy.codegen possibly sympy.codegen.rewriting.

Then there's the question of whether the logic should be a printer option or something that the user applies themselves (separation of concerns, and keeping the complexity of the printers at bay). I'm leaning toward the latter, at least that's what we ended up doing for create_expand_optimization. It is always simpler adding new helpers which apply different transformations (if the learing curve is too steep) at a later stage, than it is to revert APIs which allow for everything but the kitchen sink.

Either way optimize=True/False will likely not be fine-grained enough?

@anpandey

This comment has been minimized.

Copy link
Contributor Author

commented Jun 18, 2019

@bjodah Right now, the optimizations are exclusive to numpy. The optimization would also probably work with the Octave printer, but Octave accomplishes this with an arithmetic operator (\) instead of a function call, which seems to be unsupported by the Octave printer for now. I'm not too familiar with any of the other languages, so I'm focusing on numpy for now.

My motivation for combining the code printer and optimizations was because each language might have its own way of representing an operation (such as with Octave) and a given optimization might apply to only a certain language. Now that I think about it, we could probably abstract this away with a specialized AST node (like MatSolve) and printers for each language. This would probably also solve the issue of the fine-grainedness of optimize={True,False}.

@bjodah

This comment has been minimized.

Copy link
Member

commented Jun 18, 2019

I'm no expert in those languages either, maybe someone else can chime in there. A MatSolve node sounds like a good idea to me.

Move to representing solve as an AST node
Removes the `optimize` option introduced to the code printer in the
previous commit, instead instroducing the inverse optimization as a
top-level variable in `codegen.rewriting` that has to be applied by the
user. Also abstracts the solving operation as a `MatrixSolve` AST node
that can be handled by each code printer uniquely.
@anpandey

This comment has been minimized.

Copy link
Contributor Author

commented Jun 18, 2019

The build failed because of a circular import -- the MatrixSolve node needs to subclass MatrixExpr for matrix expressions involving it to be considered valid. I'm not exactly sure how I can approach this.

@anpandey anpandey marked this pull request as ready for review Jun 18, 2019

@anpandey anpandey changed the title Add optimization of numpy code involving matrix inverses [WIP] Add optimization of numpy code involving matrix inverses Jun 18, 2019

@bjodah

This comment has been minimized.

Copy link
Member

commented Jun 18, 2019

Instead of putting it in ast.py you can put it in another file which isn't imported early. We have C-specific nodes in cnodes.py, Fortran specific nodes in fnodes.py and C++ specific ones in cxxnodes.py

This might span multiple languages so perhaps a "common_nodes.py" or "matrix_operation_nodes.py" file?

@anpandey anpandey force-pushed the anpandey:numpy_optim branch from d658839 to 90ca7e4 Jun 18, 2019

@bjodah

This comment has been minimized.

Copy link
Member

commented Jun 20, 2019

Great work! I really like the approach in the last couple of commits, looking forward to seeing more of these kinds of transformations.

@oscarbenjamin

This comment has been minimized.

Copy link
Contributor

commented Jun 23, 2019

@bjodah is this good to merge?

@bjodah

This comment has been minimized.

Copy link
Member

commented Jun 24, 2019

The title has [WIP] in it? Other than that I think it looks good.

@anpandey anpandey changed the title [WIP] Add optimization of numpy code involving matrix inverses Add optimization of numpy code involving matrix inverses Jun 24, 2019

@anpandey

This comment has been minimized.

Copy link
Contributor Author

commented Jun 24, 2019

@bjodah I think it's ok to be merged now -- I'll address some other issues in a different PR.

@bjodah

This comment has been minimized.

Copy link
Member

commented Jun 24, 2019

Sounds good, looks like there's a merge conflict so maybe merge master into your branch?

@asmeurer

This comment has been minimized.

Copy link
Member

commented Jun 24, 2019

I've updated the release notes. This looks good to me.

@asmeurer asmeurer merged commit 6da7d01 into sympy:master Jun 24, 2019

3 checks passed

codecov/project 74.467% (target 0%)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
sympy-bot/release-notes The release notes look OK
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.