-
Notifications
You must be signed in to change notification settings - Fork 22k
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
HalfCauchy should ValueError if _validate_args #50403
Conversation
**Expected**: When I run `torch.distributions.HalfCauchy(1.0, validate_args=True).log_prob(-1)`, I expect a `ValueErro` because that is the behavior of other distributions (e.g. Beta, Bernoulli). **Actual**: No run-time error is thrown, but a `-inf` log prob is returned.
💊 CI failures summary and remediationsAs of commit edd20ab (more details on the Dr. CI page): 💚 💚 Looks good so far! There are no failures yet. 💚 💚 This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.Please report bugs/suggestions to the (internal) Dr. CI Users group. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you fixing this, @feynmanliang!
Let me fix CI; I think the tests may need a bump |
Hmm..there is a test failure in
|
Codecov Report
@@ Coverage Diff @@
## master #50403 +/- ##
==========================================
- Coverage 80.70% 80.70% -0.01%
==========================================
Files 1905 1905
Lines 206755 206757 +2
==========================================
Hits 166859 166859
- Misses 39896 39898 +2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@neerajprad has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
@neerajprad merged this pull request in fc9f013. |
Summary: Fixes #50404 Complementary to #50403 This also fixes `HalfCauchy.cdf()`, `HalfNormal.log_prob()`, `HalfNormal.cdf()` and ensures validation is not done twice. cc feynmanliang Pull Request resolved: #50492 Reviewed By: mrshenli Differential Revision: D25909541 Pulled By: neerajprad fbshipit-source-id: 35859633bf5c4fd20995182c599cbcaeb863cf29
Expected: When I run
torch.distributions.HalfCauchy(torch.tensor(1.0), validate_args=True).log_prob(-1)
, I expect aValueErro
because that is the behavior of other distributions (e.g. Beta, Bernoulli).Actual: No run-time error is thrown, but a
-inf
log prob is returned.Fixes #50404
This change is![Reviewable](https://camo.githubusercontent.com/1541c4039185914e83657d3683ec25920c672c6c5c7ab4240ee7bff601adec0b/68747470733a2f2f72657669657761626c652e696f2f7265766965775f627574746f6e2e737667)