Pyro: Renaming things before launch

Created on 27 Sep 2017  Â·  14Comments  Â·  Source: pyro-ppl/pyro

This issue is a laundry list of possible alternate names for things in Pyro.

Distributions

  • [x] #384 Rename Distribution.support() -> .enumerate_support()
  • [x] #382 DiagNormal, NormalChol -> Normal*, and maybe even an explicit distinction between univariate Normal and multivariate Normal such as Normal* and MultivariateNormal*
  • [x] Make sure param naming is uniform, eg in distributions
  • [x] #182 Settle on internal variable name convention (see #167)

General

  • [x] #387 Rename package for pypi (probably pyro-ppl) (see #150)
  • [x] Revisit names for inference operations
  • [x] KL_QP.eval_grad -> .eval_surrogate_loss (or change its behavior to return loss.grad (see #168)
  • [x] combine the relevant kl_qp implementations
  • [x] KL_QP/ELBo -> Elbo to spare Martin's fingers. (pep8 recommends Elbo) (see #74)

Triaged

  • [ ] Rename condition to observe to clarify that it conditions on values instead of arbitrary propositions being true
  • [ ] Remove observe as alias of sample (completes #29 )
  • [ ] Greek names -> meaningful names (mu, sigma -> loc, scale) (should match torch.distributions)
  • [ ] log_pdf -> sum_log_prob, batch_log_pdf -> log_pdf (to match torch.distributions api)
  • [ ] Rename LambdaPoutine to something more evocative (IndepPoutine?), especially after #184
  • [ ] map_data -> imap/map_iid (imap is used in Python2's itertools library as a map iterator.)
  • [ ] Rename *Poutine to *Handler...

(Edited to include everyone's ideas)

discussion usability

Most helpful comment

More considerations:

  • Greek names -> meaningful names (mu, sigma -> loc, scale)
  • log_pdf/log_pmf -> log_prob
  • DiagNormal, NormalChol -> Normal, and maybe even an explicit distinction between univariate Normal and multivariate Normal such as Normal and MultivariateNormal*

All 14 comments

Would map_data be clearer as map_iid?

yes! this was in the plan initially, but we never got around to it. we should do.

Would Poutine be clearer as pyro.Frame, a "probabilistic frame" in the _PYRO_STACK?

i'd be happy to rename poutines to something else, if there is consensus.... there may be strong feelings. ;)

  • KL_QP to ELBo
  • revisit names for inference operations
  • make sure param naming is uniform, eg in distributions

i agree that map_data is suboptimal. @eb8680 suggested imap which is short and to the point.

i prefer ELBO to ELBo. my fingers are not supple enough to control the shift key that finely.

unfortunately, for theological reasons the name poutine is eternally fixed. rational arguments in support of alternatives have a priori no weight. there may or may not be a cron job that defensively protects the repo against any adverserial name changes.

personally i prefer ELBO or Elbo; ELBo is the more correct capitalization but isnt consistent with how classes are named in python + hard to type. i have no skin in the poutine game nor do i have anything against it

make sure param naming is uniform

variables in general as well, we were just discussing yesterday how we use _variable differently.

action item is in #74

Haha..I would prefer ELBO -> Elbo and KL_QP -> Klqp (or, KLqp), just to be consistent with class naming conventions, and since the abbreviated form is anyways well known. Some discussion on the topic though - https://stackoverflow.com/questions/2853531/how-do-you-pep-8-name-a-class-whose-name-is-an-acronym

More considerations:

  • Greek names -> meaningful names (mu, sigma -> loc, scale)
  • log_pdf/log_pmf -> log_prob
  • DiagNormal, NormalChol -> Normal, and maybe even an explicit distinction between univariate Normal and multivariate Normal such as Normal and MultivariateNormal*

@dustinvtran agreed, especially with the first point.

All this grecophobia here...

On Thu, Sep 28, 2017 at 11:25 AM, eb8680 notifications@github.com wrote:

@dustinvtran https://github.com/dustinvtran agreed, especially with the
first point.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/uber/pyro/issues/159#issuecomment-332923181, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ABVhL9zoCkFzriiEDNKSC8J2VcnI3Zc4ks5sm-SzgaJpZM4PmKse
.

@dustinvtran good suggestions! though log_pdf comes from the numpy/scipy naming so i think that's pretty standard ....though standard doesnt imply good of course, just that everyone is used to it (like cat = concat in torch)

Added #167 and #150 to checklist

Added LambdaPoutine thanks to #184

Would Poutine be clearer as pyro.Frame, a "probabilistic frame" in the _PYRO_STACK?

i'd be happy to rename poutines to something else, if there is consensus.... there may be strong feelings. ;)

I don't have the full picture, but from my perspective renaming "poutine" to "handler" (or similar) seems likely to be helpful. (runs away...)

Added enumerate_support based on #223

Closing this since most of the renaming is done. Feel free to open issues for specific renaming.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

neerajprad picture neerajprad  Â·  4Comments

eb8680 picture eb8680  Â·  4Comments

robsalomone picture robsalomone  Â·  4Comments

fritzo picture fritzo  Â·  4Comments

null-a picture null-a  Â·  4Comments