wishart_cholesky signatures

[spinkney]

Submission Checklist

• [x] Run unit tests
• Documentation
  • [x] If a user-facing facing change was made, the documentation PR is here: https://github.com/stan-dev/docs/pull/526

Adds wishart_cholesky_lpdf and wishart_cholesky_rng.

Release notes

Added wishart_cholesky_lpdf and wishart_cholesky_rng distribution functions.

Copyright and Licensing

By submitting this pull request, the copyright holder is agreeing to license the submitted work under the BSD 3-clause license (https://opensource.org/licenses/BSD-3-Clause)

[WardBrian]

I think this distribution should also be added to the Pedantic mode warnings for using it without a cholesky matrix.

See: https://github.com/stan-dev/stanc3/blob/762f03026bfbc83007cd18692d3362bc0d0c60d0/src/analysis_and_optimization/Pedantic_dist_warnings.ml#L246 Example: https://github.com/stan-dev/stanc3/blob/762f03026bfbc83007cd18692d3362bc0d0c60d0/src/analysis_and_optimization/Pedantic_dist_warnings.ml#L423-L424

[WardBrian]

@spinkney it looks like you need to run make format; dune promote to get CI to run. The code itself looks good

[WardBrian]

Failures look like it's due to the fact that we implictly add a _log function even though this has been deprecated, and I'm guessing the stan-math PR didn't add one?

[spinkney]

that's right, I didn't add one

[WardBrian]

Yeah, we honestly shouldn’t be adding new _log signatures at this point since we’re going to remove them all so soon. Tomorrow I’ll look at re-writing how we add these signatures to let us not add it for things that didn’t have it before.

[spinkney]

will this add an lupdf signature?

[WardBrian]

All lpdf signatures are implicitly lupdf signatures as well I believe

[spinkney]

ok, honestly I wasn't sure how that was going to get done b/c they aren't in stan-math

[WardBrian]

It’s not a bad idea to add some calls of it to the test file. The way we handle lupdfs is honestly pretty poorly, when typechecking we detect unnormalized suffixes and temporarily replace them with the normalized versions to see if the signatures work

[WardBrian]

@spinkney if you merge master into this it should now not try to add the log suffix by default

[spinkney]

@WardBrian it still added _log :(

[WardBrian]

It looks like it just needs to have the test output updated

[WardBrian]

I think you still removed the log suffix from the old Wishart in the merge, which is why the latest tests failed

[WardBrian]

This is now passing. Once the docs PR gets approved we can merge - not confident enough in the theory to review the docs myself