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

store batch_shape as torch.Size object in MultivariateStudentT distribution #3099

Merged
merged 3 commits into from
Jun 2, 2022

Conversation

flo-schu
Copy link
Contributor

@flo-schu flo-schu commented Jun 1, 2022

resolves #3098
according to suggestions by @fritzo

Since this is my first contribution here, I'd be happy if you could point out, things I should improve in the PR

@flo-schu flo-schu changed the title store batch_shape as torch.Size object store batch_shape as torch.Size object in MultivariateStudentT distribution Jun 1, 2022
fritzo
fritzo previously approved these changes Jun 1, 2022
@fritzo
Copy link
Member

fritzo commented Jun 1, 2022

happy if you could point out, things I should improve in the PR

Looks great, it's super simple. One thing we do is add a "Tested" section to PR descriptions, but this would would just look like:

Tested

  • refactoring is covered by existing tests

@fritzo
Copy link
Member

fritzo commented Jun 1, 2022

Could you make format to fix the long line?

@flo-schu
Copy link
Contributor Author

flo-schu commented Jun 1, 2022

Could you make format to fix the long line?

I assume there is a shortcut to what I did?

@fritzo
Copy link
Member

fritzo commented Jun 1, 2022

I assume there is a shortcut to what I did?

All I see is the failed lint stage (LMK if you have trouble accessing this) where black complains. I don't know what exactly it's complaining about (guessing a long line), but whatever it's complaining about you should be able to fix via make format or simply black .. We do mention formatting in CONTRIBUTING.md and if you can offer any improvements to contributing docs, we welcome those contributions too 🙂

@eb8680 eb8680 merged commit 24b93f4 into pyro-ppl:dev Jun 2, 2022
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.

MultivariateStudentT distribution initializes batch_shape as tuple instead of torch.Size
3 participants