stan-dev / stanc3

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

expose inv_wishart_cholesky #1202

Closed spinkney closed 2 years ago

spinkney commented 2 years ago

Submission Checklist

Adds inv_wishart_cholesky_lpdf and inv_wishart_cholesky_rng.

Release notes

Added inv_wishart_cholesky_lpdf and inv_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 commented 2 years ago

I think you need to run make test; dune promote to update the test files after fixing that typo

spinkney commented 2 years ago

I keep getting this merlin wants to change as well

Screen Shot 2022-05-19 at 2 10 40 PM
WardBrian commented 2 years ago

That should have been fixed by #1198, have you pulled master recently?

spinkney commented 2 years ago

yea and everytime I run make I get that file change

WardBrian commented 2 years ago

oh, I see, that's undoing a change from #1197 which snuck in. Yeah, we should update to the version it wants to change to

WardBrian commented 2 years ago

Does this depend on a specific math PR? The most recent test failure was

test/expressions/tests3_test.cpp:74640:45: error: no member named 'inv_wishart_cholesky_rng' in namespace 'stan::math'; did you mean 'wishart_cholesky_rng'?
spinkney commented 2 years ago

Yes, I was trying to do the expression tests for https://github.com/stan-dev/math/pull/2713. @rok-cesnovar said the PR is good. I'm probably just confused on how to get these tests to work. It's not in stan-math yet so how can I run them?

When I run

(base) sean.pinkney@US5OMGM02GR9G math % python3 runTests.py test/expressions/ --only-function "inv_wishart_cholesky_lpdf"
make: `test/expressions/stanc' is up to date.
Error in getting signatures from stanc3!

with this stanc3 branch I get that error ^

WardBrian commented 2 years ago

If you have Jenkins permissions you can run them against specific PRs by doing to https://jenkins.flatironinstitute.org/job/Stan/job/Stanc3/job/PR-1202/build. If you don't have an account there, we can have @serban-nicusor-toptal set one up

I'll run it against that PR now!

spinkney commented 2 years ago

@WardBrian I do have an account but the pw is saved somewhere I don't have access to at the moment. I'll be back there Sunday so please run for me in the meantime

WardBrian commented 2 years ago

Oh, and expression testing is currently only for the functions, not distributions, so I think that is why that command has failed?

spinkney commented 2 years ago

If I switch branches to main and I run make, switch to the stan-math main branch, it works for

python3 runTests.py test/expressions/ --only-function "wishart_cholesky_lpdf"
WardBrian commented 2 years ago

I think you can do python3 runTests.py test/expressions/ --only-functions " inv_wishart_cholesky_lpdf(matrix, real, matrix) => real" to bypass needing stanc3 entirely, otherwise you need to copy the built stanc to the math tests folder I believe

spinkney commented 2 years ago

What I did was

Went to stan-math and cd'd into the make directory. I added a file named local and inside that I added STANC3=/location/to/stanc3

I tried what you posted and I'm getting 6 errors

test/expressions/tests0_test.cpp:20:45: error: no member named 'inv_wishart_cholesky_lpdf' in namespace 'stan::math'
auto result8 = stan::math::eval(stan::math::inv_wishart_cholesky_lpdf(matrix0_expr3,real1,matrix2_expr4));
                                ~~~~~~~~~~~~^
test/expressions/tests0_test.cpp:21:45: error: no member named 'inv_wishart_cholesky_lpdf' in namespace 'stan::math'
auto result9 = stan::math::eval(stan::math::inv_wishart_cholesky_lpdf(matrix5,real6,matrix7));
                                ~~~~~~~~~~~~^
test/expressions/tests0_test.cpp:45:45: error: no member named 'inv_wishart_cholesky_lpdf' in namespace 'stan::math'
auto result8 = stan::math::eval(stan::math::inv_wishart_cholesky_lpdf(matrix0_expr3,real1,matrix2_expr4));
                                ~~~~~~~~~~~~^
test/expressions/tests0_test.cpp:46:45: error: no member named 'inv_wishart_cholesky_lpdf' in namespace 'stan::math'
auto result9 = stan::math::eval(stan::math::inv_wishart_cholesky_lpdf(matrix5,real6,matrix7));
                                ~~~~~~~~~~~~^
test/expressions/tests0_test.cpp:78:45: error: no member named 'inv_wishart_cholesky_lpdf' in namespace 'stan::math'
auto result8 = stan::math::eval(stan::math::inv_wishart_cholesky_lpdf(matrix0_expr3,real1,matrix2_expr4));
                                ~~~~~~~~~~~~^
test/expressions/tests0_test.cpp:79:45: error: no member named 'inv_wishart_cholesky_lpdf' in namespace 'stan::math'
auto result9 = stan::math::eval(stan::math::inv_wishart_cholesky_lpdf(matrix5,real6,matrix7));
                                ~~~~~~~~~~~~^
6 errors generated.
WardBrian commented 2 years ago

Is that in your PR or in the develop branch? I can’t imagine why it wouldn’t be able to find the function

I’m re-running against the math PR now, let’s see if the issue shows up in the CI

spinkney commented 2 years ago

I don't know what the issues are that I'm having. I'm on the pr. Anyway, this passed!

WardBrian commented 2 years ago

Ping me once the relevant math PR is merged and the docs are reviewed and we can get this in

spinkney commented 2 years ago

@WardBrian math and docs are merged!