Skip to content
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

Make sure all distributions support AD where possible #674

Closed
mohamed82008 opened this issue Feb 4, 2019 · 7 comments
Closed

Make sure all distributions support AD where possible #674

mohamed82008 opened this issue Feb 4, 2019 · 7 comments
Assignees
Milestone

Comments

@mohamed82008
Copy link
Member

The following link shows the list of distributions that StatsFuns uses ccall for: https://github.com/JuliaStats/StatsFuns.jl/blob/7473d0d8f7c5b2f4a53f0625ada5f2323c84ffe4/src/rmath.jl#L99. We should make AD workarounds for all of these.

Supersedes #673

@xukai92
Copy link
Member

xukai92 commented Feb 7, 2019

There are some distributions in this list which already have callback functions, e.g. beta (https://github.com/JuliaStats/StatsFuns.jl/blob/7473d0d8f7c5b2f4a53f0625ada5f2323c84ffe4/src/distrs/beta.jl). I actually implemented those in StatsFun for the AD purpose in Turing so I'm happy to work on this issue on the StatsFuns side (they gave quite good review on numerical stability).

I think you also implemented some on the Turing side, e.g. https://github.com/TuringLang/Turing.jl/pull/664/files. Maybe we should port them to StatsFuns (and DiffRules) as well. How do you think?

@mohamed82008
Copy link
Member Author

@xukai92 Yes, StatsFuns or DiffRules with StatsFuns in a require block are the more stable solutions here (no type piracy).

@yebai
Copy link
Member

yebai commented Mar 9, 2019

@yebai yebai added this to the 0.7 milestone Mar 9, 2019
@yebai yebai pinned this issue Mar 9, 2019
@xukai92
Copy link
Member

xukai92 commented Mar 9, 2019

We also need to test both forward and reverse mode (different AD implementation). And vector operations (different in bijection mapping) for all.

@yebai
Copy link
Member

yebai commented Mar 28, 2019

Currently, the following distributions need a generic Julia implementation for pdf and logpdf,

  • hyper hyper ms mf n (discrete)
  • nbeta nbeta α β λ (obsolete, not exported by Distributions.jl)
  • nchisq nchisq k λ (obsolete, not exported by Distributions.jl)
  • nfdist k1 k2 λ (obsolete, not exported by Distributions.jl)
  • ntdist k λ (obsolete, not exported by Distributions.jl)

This can be done in a similar way to JuliaStats/StatsFuns.jl#21, or #664. We should also consider moving the hotfixes for pois, nbinom and binom into StatsFuns.jl/src/distrs/.

@yebai
Copy link
Member

yebai commented Mar 28, 2019

Here's the full list of distributions from Distributions.jl, but without FluxAD support:

@mohamed82008
Copy link
Member Author

This seems to be fixed now. I will close the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants