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: Retain continuous approximation of Poisson in torch v1.8.0 #1351

Merged
merged 9 commits into from
Mar 5, 2021

Conversation

matthewfeickert
Copy link
Member

@matthewfeickert matthewfeickert commented Mar 5, 2021

Description

Resolves #1348

To retain the continuous approximation of the Poisson, set validate_args=False in instantiations of the torch.distributions.Poisson class.

Checklist Before Requesting Reviewer

  • Tests are passing
  • "WIP" removed from the title of the pull request
  • Selected an Assignee for the PR to be responsible for the log summary

Before Merging

For the PR Assignees:

  • Summarize commit messages into a comprehensive review of the PR
* Add validate_args=False to instantiations of the torch.distributions.Poisson class in PyTorch backend to allow for the continuous approximation of the Poisson distribution in torch v1.8+
* Ensure tensor value arguments for torch.distributions.Normal
* Check that ValueErrors are raised by PyTorch v1.8+ when the validations of validate_args are violated

@matthewfeickert matthewfeickert added the fix A bug fix label Mar 5, 2021
@matthewfeickert matthewfeickert self-assigned this Mar 5, 2021
tests/test_tensor.py Outdated Show resolved Hide resolved
@matthewfeickert
Copy link
Member Author

@lukasheinrich @kratsg I'm still thinking on this with regards to how we can make this better / cleaner. Though I welcome your thoughts and input now.

@codecov
Copy link

codecov bot commented Mar 5, 2021

Codecov Report

Merging #1351 (3040eac) into master (2c10be3) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1351   +/-   ##
=======================================
  Coverage   97.50%   97.50%           
=======================================
  Files          63       63           
  Lines        3762     3768    +6     
  Branches      538      538           
=======================================
+ Hits         3668     3674    +6     
  Misses         55       55           
  Partials       39       39           
Flag Coverage Δ
unittests 97.50% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/pyhf/tensor/pytorch_backend.py 98.27% <100.00%> (+0.09%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2c10be3...3040eac. Read the comment docs.

@matthewfeickert matthewfeickert added the tests pytest label Mar 5, 2021
@matthewfeickert matthewfeickert requested a review from kratsg March 5, 2021 15:27
@matthewfeickert matthewfeickert marked this pull request as ready for review March 5, 2021 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix A bug fix tests pytest
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PyTorch v1.8.0 breaks public API
3 participants