- 
                Notifications
    
You must be signed in to change notification settings  - Fork 230
 
          Add note for filldist and Index variables
          #1437
        
          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
          Codecov Report
 @@           Coverage Diff           @@
##           master    #1437   +/-   ##
=======================================
  Coverage   65.56%   65.56%           
=======================================
  Files          25       25           
  Lines        1661     1661           
=======================================
  Hits         1089     1089           
  Misses        572      572           Continue to review full report at Codecov. 
  | 
    
| 
               | 
          ||
| ### Indexed Group Variables | ||
| 
               | 
          ||
| The function `filldist(distribution,n)` provides a simplified interface for defining a set of model variables that share the same structure, but vary by group. | 
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.
Maybe this could be formulated in a different way? IMO it sounded a bit like filldist is designed specifically for this purpose, even though that's not the case and it is just a general way of creating a product distribution, in which all single distributions are the same and that allows also multivariate distributions (in contrast to Product in Distributions).
It would be good to mention arraydist as well in case they not all share the same structure.
Maybe something like, "if you want to model a set of variables that share the same structure but vary by group, you can use filldist and arraydist."
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.
Yes, I agree. Maybe there could be two examples, one with and one without filldist or arraydist?
| 
           Ping  | 
    
| 
           Would someone mind if I reached out to them on slack to discuss the proposed edits in more detail? My Turing et al familiarity is low and the PR was intended to be a simple example of something that wasn't documented AFAICT. I don't mind trying to get this to completion but lack familiarity with the additional methods requested to include above. I would be able to try and do this in the next couple of weeks. I'd like to learn and contribute but would need a little bit more fudge to bring this PR to a close.  | 
    
| 
           Sure, you can contact anyone you like or better write in the #turing channel on Slack.  | 
    
| 
           @trappmartin Would you like to give it a try and improve the docs in this PR?  | 
    
| 
           I can have a look at it after ICML deadline.  | 
    
| 
           No problem - it's not urgent but seems like low-hanging fruit.  | 
    
| 
           Ping @trappmartin  | 
    
| 
           I'll look into it.  | 
    
| 
           Closed in favour of #1751  | 
    
…1751) * Add note for `filldist` and Index variables closes #1422 * updated docs for filldist and arraydist * Update docs/src/using-turing/guide.md Co-authored-by: Rik Huijzer <[email protected]> Co-authored-by: alecloudenback <[email protected]> Co-authored-by: Rik Huijzer <[email protected]>
closes #1422