-
-
Notifications
You must be signed in to change notification settings - Fork 198
New name wiener_lc(c)df_defective #3258
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
New name wiener_lc(c)df_defective #3258
Conversation
WardBrian
left a comment
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.
Thanks @Franzi2114
If you can provide me the Stan signatures of these new functions I can put together the compiler PR rather quickly!
If you have the time, it would also be great to at least add these to the functions reference documentation https://github.com/stan-dev/docs/blob/master/src/functions-reference/positive_lower-bounded_distributions.qmd#L158
Do you mean this one from the With the old name I had the following line: Would it now be this one? |
What is this? Are here all possible function calls listed? Why is it Actually, all functions are implemented in the way that the user is able to set the precision of the partial derivatives. This shall increase the speed of the computations. This is implemented like this, with the last parameter for the precision: Is this already possible with these signatures or should we also add an entry like: |
I don't mean in OCaml code, just the valid signatures. Right now, here's whats there for wiener_lpdf with 6 or 8 arguments Looking at this PR, it looks like the cdf/ccdf takes either 5 or 8 arguments, rather than 6 or 8. Is that correct?
This is the document that gets turned into https://mc-stan.org/docs/functions-reference/positive_lower-bounded_distributions.html#wiener-first-passage-time-distribution In particular, I would mostly need your help writing out the math and the names for the arguments, etc. The specially formatted comments etc I can handle (they're used for things like generating an index of all functions) |
This is not currently implemented by the compiler, but it would be possible to expose to the user with new overloads. We're lucky that we can still tell them apart just by length (if one of the non-precision overloads had 7 arguments, then 7+precision = 8 and that would be ambiguous with the 8-argument non-precision) |
This comment was marked as off-topic.
This comment was marked as off-topic.
|
Ok, so the following function calls should work: In both variants with This is analog with the cdf/ccdf functions: In both variants with Does this help? |
|
That sounds good! @Franzi2114 I'm getting an unused variable warning for |
|
@Franzi2114, |
Oh, indeed! Yes, this should also be 1e-4 as in the other function. Do you want to add this or shall I do another PR? |
I just checked this. This is not used in the code and can therefore be deleted in both If you want, I can do another PR where I add the precision default value and delete these two unused variables. Or do you want to do it, @WardBrian? |
|
I’m happy to do it, there are few other uninteresting testing-related changes that are needed that I will be doing as well. Thanks for confirming! |
Summary
This is a short PR building upon the PRs to add the
wiener_pdf(PR #2822),wiener_cdfandwiener_ccdf(PR #3042) functions and refers to issue #3256.The only thing that this PR changes is the names of the cdf and ccdf functions as they are implemented defectively, which means that the functions do not integrate to 1, but to the probability to hit one or the other response boundary. Therefore, the standard way of calculating truncated and censored models with the truncation operator (
T[,]) yields wrong results and the manual way to compute such models should be chosen.Papers
Release notes
Change names for
wiener_lc(c)dftowiener_lc(c)df_defectiveChecklist
Copyright holder: Franziska Henrich
The copyright holder is typically you or your assignee, such as a university or company. By submitting this pull request, the copyright holder is agreeing to the license the submitted work under the following licenses:
- Code: BSD 3-clause (https://opensource.org/licenses/BSD-3-Clause)
- Documentation: CC-BY 4.0 (https://creativecommons.org/licenses/by/4.0/)
the basic tests are passing
./runTests.py test/unit)make test-headers)make test-math-dependencies)make doxygen)make cpplint)the code is written in idiomatic C++ and changes are documented in the doxygen
the new changes are tested