-
Notifications
You must be signed in to change notification settings - Fork 421
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
Add Convolve for DiscreteNonParametric (Redux) #1850
Conversation
DiscreteNonParametric convolution has a very nice trivial closed form. It was not implemented. This pull request implements it.
Co-authored-by: David Widmann <[email protected]>
Co-authored-by: David Widmann <[email protected]>
Doesn't preserve the type of the Vector, but perhaps this is better .... Co-authored-by: David Widmann <[email protected]>
use functions to access the support and probabilities, and write as one loop. Co-authored-by: David Widmann <[email protected]>
Removed check args: We know the convovultion is a proper distribution.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1850 +/- ##
==========================================
+ Coverage 85.94% 85.95% +0.01%
==========================================
Files 144 144
Lines 8656 8667 +11
==========================================
+ Hits 7439 7450 +11
Misses 1217 1217 ☔ View full report in Codecov by Sentry. |
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.
I had an idea for a few performance improvements but I think it's better to play around with these in a follow-up PR. Thank you!
Originally implemented by @iampritishpatil here: #1523
This PR additionally implements a number of test improvements suggested by @devmotion. Happy to merge this or complete the original PR.
Thanks!