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 Op corresponding to scipy.linalg.solve_discrete_are #417

Merged
merged 5 commits into from
Aug 25, 2023

Conversation

jessegrabowski
Copy link
Member

@jessegrabowski jessegrabowski commented Aug 23, 2023

Motivation for these changes

Move solve_discrete_are from pymc_experimental/statespace/pytensor_scipy.py to here.

I also did a few cleanups in tensor.slinalg.py:

  • Removed unused helper function is_complex_obj
  • Removed unused solvetriangular instance of SolveTriangular (the SolveTriangular Op is instead accessed through the solve_triangular wrapper function)
  • Changed return type hint on Op wrapper functions from TensorVariable to Variable to appease the linter
  • Added solve_discrete_lyapunov, solve_cotinuous_lyapunov, solve_discrete_are, and solve_triangular to __all__, so there are no more "hidden functions" in nlinalg

Implementation details

Op is just a wrapper around scipy.linalg.solve_discrete_are, with gradients taken from Kao and Hennequin (2020), https://arxiv.org/pdf/2011.11430.pdf

The use-case is solving matrix quadratic equations. It pops up in state space models (the steady state covariance matrix comes from the solution to such an equation). It can also be used to compute the policy function for Bellman equations with linear-quadratic loss function (also relevant to state space modeling).

Checklist

Major / Breaking Changes

  • ...

New features

  • ...

Bugfixes

  • ...

Documentation

  • ...

Maintenance

  • ...

pytensor/tensor/slinalg.py Outdated Show resolved Hide resolved
pytensor/tensor/slinalg.py Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Aug 23, 2023

Codecov Report

Merging #417 (d2d1c15) into main (d9b494d) will decrease coverage by 0.01%.
Report is 13 commits behind head on main.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #417      +/-   ##
==========================================
- Coverage   80.78%   80.77%   -0.01%     
==========================================
  Files         156      156              
  Lines       45483    45571      +88     
  Branches    11139    11152      +13     
==========================================
+ Hits        36742    36812      +70     
- Misses       6532     6552      +20     
+ Partials     2209     2207       -2     
Files Changed Coverage Δ
pytensor/tensor/slinalg.py 94.91% <100.00%> (+1.37%) ⬆️

... and 9 files with indirect coverage changes

@jessegrabowski
Copy link
Member Author

Does the solve = Solve() here do anything? It's immediately shadowed by def solve in the next line.

@ricardoV94
Copy link
Member

ricardoV94 commented Aug 24, 2023

Does the solve = Solve() here do anything? It's immediately shadowed by def solve in the next line.

No. I guess it was left accidentally when the parametrized helper was implemented

@jessegrabowski jessegrabowski merged commit 288a3f3 into pymc-devs:main Aug 25, 2023
52 checks passed
@jessegrabowski jessegrabowski deleted the solve_discrete_are branch August 25, 2023 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants