-
-
Notifications
You must be signed in to change notification settings - Fork 70
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[CmdStan 2.31] Add log_prob
function to model class
#637
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #637 +/- ##
===========================================
+ Coverage 80.13% 80.16% +0.03%
===========================================
Files 69 69
Lines 10335 10392 +57
===========================================
+ Hits 8282 8331 +49
- Misses 2053 2061 +8
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There should a function that outputs wanted parameters, their shape and dtype. Also validation function is needed.
This needs ndarray or unravelled table format parameters?
Not sure I follow what you're requesting here. The input is the same as for |
Ok, yeah maybe what I describe is more like a helper functions for users so they are aware of what kind of input is needed. Init works also with a subset of parameters? So users are fine with giving it wrong parameters. |
The content isn't necessarily the same as inits, only the format. You will need to specify every parameter or an error will be thrown (see the test The output is harder to interpret, since it is on the unconstrained space, but I'm not sure if CmdStan exposes the kind of information needed for a helper function which would make it any clearer |
Yeah true. stanc has the --info or something similar but I don't think executable has access to these. |
perhaps we should wait on adding this to CmdStanPy? cf stan-dev/cmdstan#1133 who is using this feature? would bridgestan be an alternative? |
There are basically 0 situations where this is preferable to bridgestan except for the fact that it’s easily accessible. The only legit uses IMO are for debugging and the like. I know Bob and I did some problems where we were optimizing and wanted to check that the gradient at the true solution was really 0 As for waiting, the only thing that issue would change here is it would allow you to pass a CSV, which doesn’t require any code changes here, just doc. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are we only exposing log_prob given inputs on the constrained scale?
file can be Rdump JSON or CSV, correct?
Unless/until we expose a way from cmdstan to constrain and unconstrain, I think it only really makes sense |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Submission Checklist
Summary
Closes #593. Like with #634, this relies on 2.31 and for now will be tested with release candidates
I am not entirely sure if/how we want to expose this, but here's what I've done for now:
Signature:
This takes in (constrained) parameters in the same format as the
inits
argument, and data the same as for other functions. It directly returns a dataframe rather than a larger object, since it is exactly one set of outputs and the diagnostic information is not important.I've added a big warning:
Should we mention things users might want to use, like BridgeStan?
Copyright and Licensing
Please list the copyright holder for the work you are submitting (this will be you or your assignee, such as a university or company):
Simons Foundation
By submitting this pull request, the copyright holder is agreeing to license the submitted work under the following licenses: