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

Fix LocScaleReparam, log params in backtest() #2365

Merged
merged 2 commits into from
Mar 13, 2020
Merged

Conversation

fritzo
Copy link
Member

@fritzo fritzo commented Mar 13, 2020

  1. Fixes a naming bug in LocScaleReparam whereby all loc-scale-reparameterized sites shared a single centeredness parameter. After this PR each such site learns a separate centeredness parameter (as originally intended).

  2. Also adds param logging to contrib.forecast.evaluate.backtest(), which logging led me to discover the loc-scale bug. After this PR all scalar params will be printed and saved (as floats) to backtest results. My immediate motivation is to inspect global parameters stability, skew and df of Stable and StudentT distributions.

Tested

  • covered by existing tests
  • ran locally and examined logging output

if isinstance(result[name], (int, float)):
logger.debug("{} = {}".format(name, result[name]))
for name, value in pyro.get_param_store().items():
if value.numel() == 1:
Copy link
Collaborator

Choose a reason for hiding this comment

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

make the numel boundary toggleable so that users have an easy way to inspect params? e.g. they can even choose inf

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm trying to avoid logging any Tensors, instead logging only Python objects. One reason for this design choice is that a single big tensor renders the entire data structure unreadable (visually, at a terminal). One option that would still satisfy this design constraint is to log scalar statistics of each non-scalar tensor, e.g. min, max, median, mean. That seems like a reasonable feature for a follow-up PR.

Copy link
Member

@fehiepsi fehiepsi left a comment

Choose a reason for hiding this comment

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

LGTM.

@fehiepsi fehiepsi merged commit 7e20031 into dev Mar 13, 2020
@fritzo fritzo deleted the fix-loc-scale-reparam branch April 28, 2020 01:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants