Expose multivariate normal method argument in post-estimation tasks#484
Merged
Conversation
…s when doing post-estimation tasks
ricardoV94
approved these changes
May 28, 2025
| group: str, | ||
| random_seed: RandomState | None = None, | ||
| data: pt.TensorLike | None = None, | ||
| method: str = "svd", |
Member
There was a problem hiding this comment.
do you want to type hint the options and/or mention them on the docstrings? Potentially mention the reason for choosing some over the other?
Member
Author
There was a problem hiding this comment.
Just pushed this without having read this comment, so yes :)
There was a problem hiding this comment.
Pull Request Overview
This PR exposes the internal multivariate normal method argument (default "svd") in post-estimation sampling, forecasting, and related state‐space APIs, allowing users to choose faster but potentially less robust algorithms like "cholesky" or "eig".
- Added
method: str = "svd"parameters to distribution constructors (__new__,dist,rv_op) infilters/distributions.pyand to public sampling, forecasting, and IRF methods incore/statespace.py. - Propagated the
methodargument through all internalpm.MvNormal.dist(..., method=...)calls. - Updated docstrings in
core/statespace.pyto describe the newmethodparameter and its allowed values.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| pymc_extras/statespace/filters/distributions.py | Expose method argument in distribution constructors and propagate to MvNormal |
| pymc_extras/statespace/core/statespace.py | Add method parameter to sampling, forecasting, and IRF methods with docs |
Comments suppressed due to low confidence (1)
pymc_extras/statespace/filters/distributions.py:213
- Add tests for multiple
methodvalues (e.g., 'cholesky', 'eig') in thepm.MvNormalcalls inside the scan to ensure the new argument is correctly applied.
mu=0, cov=Q, rng=rng, method=method
andreacate
pushed a commit
to andreacate/pymc-extras
that referenced
this pull request
Jul 22, 2025
…pymc-devs#484) * Expose `method` argument to MvNormals used in statespace distributions when doing post-estimation tasks * Use keyword arguments when calling post-estimation functions internally * Fix typo, more clear argument name * improve type hint * Add `method` to `build_statespace_graph` and include some general advice. * Fix incorrect signature in call to _sample_unconditional
andreacate
pushed a commit
to andreacate/pymc-extras
that referenced
this pull request
Jul 25, 2025
…pymc-devs#484) * Expose `method` argument to MvNormals used in statespace distributions when doing post-estimation tasks * Use keyword arguments when calling post-estimation functions internally * Fix typo, more clear argument name * improve type hint * Add `method` to `build_statespace_graph` and include some general advice. * Fix incorrect signature in call to _sample_unconditional
velochy
pushed a commit
to velochy/pymc-extras
that referenced
this pull request
Feb 20, 2026
…pymc-devs#484) * Expose `method` argument to MvNormals used in statespace distributions when doing post-estimation tasks * Use keyword arguments when calling post-estimation functions internally * Fix typo, more clear argument name * improve type hint * Add `method` to `build_statespace_graph` and include some general advice. * Fix incorrect signature in call to _sample_unconditional
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Post-estimation sampling tasks, especially
sample_conditional_posterior, can be really slow for long time series. One reason for this is that I had set themethodargument of all the multivariate normal distributions internally to besvd, which is the most robust option. There are cases, however, where one knows the model is well behaved, so you can drop back to something faster (likecholesky) without any problem. This PR allows this choice.The default is still
svd. Ideally we'd check a couple covariance matrices from the providedidataand try to give the user a smart default, but I don't want to do that much work here.