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.

Sign up for GitHub

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

Updated LocScaleReparam's documentation #1599

Merged
merged 2 commits into from
Jun 1, 2023
Merged

Updated LocScaleReparam's documentation #1599

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
26627f4
Update reparam.py
Madhav-Kanda Jun 1, 2023
11818f9
Making suggested changes
Madhav-Kanda Jun 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions numpyro/infer/reparam.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,7 @@ class LocScaleReparam(Reparam):
1. *Automatic Reparameterisation of Probabilistic Programs*,
Maria I. Gorinova, Dave Moore, Matthew D. Hoffman (2019)

:param float centered: optional centered parameter. If None (default) learn
a per-site per-element centering parameter in ``[0,1]``. If 0, fully
:param float centered: optional centered parameter. If None (default) considers centered parameter to be 0.5. If 0, fully
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you change this to something like:

    :param float centered: optional centered parameter. If None (default) learn
        a per-site per-element centering parameter in ``[0,1]`` initialized at value 0.5.
        To sample the parameter, consider using :class:`~numpyro.handlers.lift` handler with a
        prior like ``Uniform(0, 1)`` to cast the parameter to a latent variable. If 0, fully

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, I would change this, but based on my understanding of the current implementation, it would not learn the center param even when we keep it None. It would simply keep the centered param as 0.5. To learn the correct parameter based on Maria Gorinova's paper, we will have to use Variational Inference. Please correct me if I need to correct something. Thanks for the quick response.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's why we say this is a parameter, not a latent variable. When using MCMC, you are drawing samples of latent variables. When you say "optimizing", it typically means that you are using inference methods with optimizers like SGD, Adam and gradient descent to optimize the corresponding parameters. When you say "sampling", it means that you are using MCMC to draw samples of latent "random variable".

Copy link
Member

@fehiepsi fehiepsi Jun 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note that in numpyro, MCMC does not sample "parameters", it only samples latent variables. If you think the word "parameters" is not clear enough, I think you can change it to something like

If None (default) create a ``numpyro.param`` site for a per-site per-element centering parameter ...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, okay. Thank you!

decenter the distribution; if 1, preserve the centered distribution
unchanged.
:param shape_params: list of additional parameter names to copy unchanged from
Expand Down