-
-
Notifications
You must be signed in to change notification settings - Fork 199
Create a class to supersede operands_and_partials for N-ary operations #2841
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
Conversation
…rtials for N-ary distributions
…e make_partials_propogation
|
Staring at the code for using edge<0>(ops_partials).partials_ = elt_divide(-P, Pi_cl);It's possible to just call partials<0>(ops_partials) = elt_divide(-P, Pi_cl); |
…h into feature/tuple-ops-partials3
|
@andrjohns would you mind taking a look at this? I've looked over all of the Not super high priority. This is mostly a nice cleanup. There may be some small optimizations for cases with only 1 or 2 parameters since we only store as many edges as we need now (and then only do reverse passes for those 1 or 2 parameters as well). Though I think we would only see a noticeable speedup when applying the scalar form of the lpdf many times. |
andrjohns
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.
You had me scared with the size of the diff, but overall the changes are pretty clean and minor! Just a few comments/suggestions
andrjohns
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.
All LGTM! Feel free to merge if you don't have any other changes
Summary
This creates a class
propagate_partialsthat is a drop in replacement foroperands_and_partials. A new class was created so that stan math can stay backwards compatible. Underneath the hoodpropagate_partialsis very similar tooperands_and_partials, but now each edge is stored inside of a tuple. This has a small benefit of saving some memory relative tooperands_and_partialssince we previously allocated edges for 5 operations even if we only needed one. The main benefit is that this can support any number of operations and is a drop in replacement foroperands_and_partials.This change also makes for nicer code, where previously we had to include a lengthy template for
operands_and_partialswe can now just useautoand call themake_partials_propagator()function which then creates and returns the appropriatepartials_propagatorclass. This is similar to the standard librariesmake_tuple()function.Instead of
operands_and_partials'sedge1_andedge2_thepartials_propagatorclass has an associated function callededge()which is used in the same way asstd::tuple'sstd::get()functionTests
Since this is a drop in replacement for
operands_and_partialsI took all of the tests for that classe and wrote the corresponding ones forpartials_propagator. They can be run viaSide Effects
I changed all of the distributions to use
make_partial_propagatorso a lot of files are changed. I did this because I wanted to make sure everything worked and the easiest way was to swap them out for one another.imo the files that really need looked at thoroughly are the
operands_and_partialsandpartial_propagatorones in prim/fwd/rev. I'll double check the distributions match correctly.Release notes
Allow for for N-ary partials for distributions
Checklist
Math issue Adding 7-Parameter Drift Diffusion Model (DDM) PDF with partial derivatives to Stan Math #2682
Copyright holder: Steve Bronder
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