search

stan-dev / stanc3

The Stan transpiler (from Stan to C++ and beyond).
BSD 3-Clause "New" or "Revised" License
142 stars 46 forks source link

expose log determinant spd #1178

Closed spinkney closed 2 years ago

spinkney commented 2 years ago

Submission Checklist

Release notes

Added log_determinant_spd which is faster when the input matrix is known to be symmetric and positive definite.

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)

rok-cesnovar commented 2 years ago

The tests now pass with the math PR branch.

The PR looks good, just needs a docs PR with some basic function docs and this should be good to go.

spinkney commented 2 years ago

@rok-cesnovar should I change to SoA or keep it as AoS?

spinkney commented 2 years ago

Also the doc PR is https://github.com/stan-dev/docs/pull/520

rok-cesnovar commented 2 years ago

Great! If the function supports “varmat”, lets mark it SoA, else we keep it AoS.

spinkney commented 2 years ago

Sorry but how do I know if it supports varmat?

rok-cesnovar commented 2 years ago

Mark it as SoA and we can let Jenkins tell us :)

rok-cesnovar commented 2 years ago

The Jenkinsfile isn't set up for this case, so it didn't start the required tests, but I verified locally that varmat is supported.

The PR looks great, I will officially approve once the Math PR is merged.

WardBrian commented 2 years ago

This is dependent on https://github.com/stan-dev/math/pull/2719, correct?

spinkney commented 2 years ago

This is dependent on stan-dev/math#2719, correct?

yes and I believe I've addressed @SteveBronder's review. Once he approves #2719 I'll tag @WardBrian here to review.