Skip to content

Add AePPL logp/logcdf references for all Distributions that are missing them #5329

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

Closed
canyon289 opened this issue Jan 9, 2022 · 3 comments · Fixed by #6272
Closed

Add AePPL logp/logcdf references for all Distributions that are missing them #5329

canyon289 opened this issue Jan 9, 2022 · 3 comments · Fixed by #6272
Labels

Comments

@canyon289
Copy link
Member

canyon289 commented Jan 9, 2022

Description of your problem

Many of the distributions in PyMC call another library, AePPL, for their logp and logcdf calculation.

Please add references to the AePPL log P library. See PR below for an example of the code changes needed, and where to find the AePPL library

#5327

@canyon289 canyon289 changed the title Add Aeppl Log P references for all Distributions that are missing them Add AePPL logp references for all Distributions that are missing them Jan 9, 2022
@ricardoV94 ricardoV94 changed the title Add AePPL logp references for all Distributions that are missing them Add AePPL logp/logcdf references for all Distributions that are missing them Jan 9, 2022
@OriolAbril
Copy link
Member

These should probably use sphinx cross-references. I don't completely understand aeppl, it's scope and audience nor the relation pymc-aeppl, and I'm willing to be convinced otherwise in this scenario.

IMO, there are two reasons for the comment above:

  1. in general the average user shouldn't need to read the code at all. It is great to have the source code easily available from the api page (with the source button) and I use it quite often, but that should be a thing aimed mostly at power users and contributors, otherwise it is a failure of the documentation
  2. urls to documentation pages of other python libraries are brittle and in addition are generally less specific than sphinx cross references. In the template PR, we could have use {func}`aeppl.logprob.exponential_logprob` to point direclty to the right api page (which could then have a [source] button and so on), now it links only to the logprob file.

Note: Point 2 especially (but related to both) requires that aeppl has html documentation served somewhere. https://aesara-devs.github.io/aeppl/index.html exists but it seems to have last been updated on August. I can help (not do it myself) setting up documentation for aeppl, configuring readthedocs if that is something of interest to aesara-devs (cc @ricardoV94) and in fact I already reviewed aesara-devs/aeppl#52. If we can rely on aeppl having up to date documentation published then a link to relevant file in github is ok.

@purna135
Copy link
Member

purna135 commented Mar 2, 2022

Is it okay if I work on this?

@OriolAbril
Copy link
Member

I think it's best to wait a bit on this until we figure out aeppl docs and how to better integrate the two documentations. I'm removing the labels for now.

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

Successfully merging a pull request may close this issue.

4 participants