Skip to content

Conversation

InfProbSciX
Copy link
Contributor

References to other Issues or PRs

Fixes #21635

Brief description of what is fixed or changed

The pdf method for the MultivariateNormalDistribution has been updated:

  • to use MatrixElement to return the [0, 0] value of the density calculation, that returns a [1, 1] matrix. This has been changed from accessing the element directly using [0], as this throws an error when certain arguments are symbolic.
  • to check if the input is a single matrix, if not, then coerce the inputs to an ImmutableMatrix. I've added this because passing a matrix with a symbolic number of elements causes the argument to be coerced to a 1x1 matrix, with the original matrix at the [0, 0] index.

Other comments

While running tests, I noticed that the test_sample_pymc3 test breaks (I have pymc3 installed). This is also true for the code on master and is unrelated to this PR.

Release Notes

  • stats
    • Fixed a bug regarding symbolic arguments to the multivariate normal distribution.

@sympy-bot
Copy link

sympy-bot commented Jun 22, 2021

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

  • stats
    • Fixed a bug regarding symbolic arguments to the multivariate normal distribution. (#21650 by @InfProbSciX)

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

Click here to see the pull request description that was parsed.
#### References to other Issues or PRs
Fixes #21635

#### Brief description of what is fixed or changed
The `pdf` method for the `MultivariateNormalDistribution` has been updated:
 * to use `MatrixElement` to return the `[0, 0]` value of the density calculation, that returns a `[1, 1]` matrix. This has been changed from accessing the element directly using `[0]`, as this throws an error when certain arguments are symbolic.
 * to check if the input is a single matrix, if not, _*then*_ coerce the inputs to an `ImmutableMatrix`. I've added this because passing a matrix with a symbolic number of elements causes the argument to be coerced to a `1x1` matrix, with the original matrix at the `[0, 0]` index.

#### Other comments

While running tests, I noticed that the `test_sample_pymc3` test breaks (I have pymc3 installed). This is also true for the code on master and is unrelated to this PR.

#### Release Notes

<!-- BEGIN RELEASE NOTES -->
* stats
  * Fixed a bug regarding symbolic arguments to the multivariate normal distribution.
<!-- END RELEASE NOTES -->

Update

The release notes on the wiki have been updated.

@oscarbenjamin
Copy link
Collaborator

@czgdp1807 @Upabjojr can you review this?

@czgdp1807
Copy link
Member

czgdp1807 commented Jul 1, 2021

If possible (like if it isn't too tricky to do) add an example to the docs as well for this case. Thanks. Rest if fine.

@InfProbSciX
Copy link
Contributor Author

@czgdp1807 All done.

@InfProbSciX
Copy link
Contributor Author

@oscarbenjamin @czgdp1807 @Abdullahjavednesar Hello everyone - just following up to see if anything more needs to be done.

@oscarbenjamin
Copy link
Collaborator

Looks good, thanks!

@oscarbenjamin oscarbenjamin enabled auto-merge July 11, 2021 18:16
@oscarbenjamin oscarbenjamin merged commit 3197699 into sympy:master Jul 11, 2021
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.

MultivariateNormal density calculation fails when matrix params are used
5 participants