-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Logprob derivation of Min for Discrete IID distributions #6968
Conversation
b941a8b
to
1d788c0
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #6968 +/- ##
==========================================
- Coverage 92.21% 83.57% -8.64%
==========================================
Files 101 101
Lines 16912 16937 +25
==========================================
- Hits 15595 14155 -1440
- Misses 1317 2782 +1465
|
1d788c0
to
f5c5c9c
Compare
Maybe a good test for discrete would be min/max(Bernoulli)? def test_min_max_bernoulli():
p = 0.7
q = 1 - p
n = 3
x = pm.Bernoulli.dist(p=p, shape=(n,))
value = pt.scalar("value", dtype=int)
max_logp_fn = pytensor.function([value], pm.logp(pt.max(x), value))
np.testing.assert_allclose(max_logp_fn(0), np.log(q ** n))
np.testing.assert_allclose(max_logp_fn(1), np.log(1 - q ** n))
min_logp_fn = pytensor.function([value], pm.logp(pt.min(x), value))
np.testing.assert_allclose(min_logp_fn(1), np.log(p ** n))
np.testing.assert_allclose(min_logp_fn(0), np.log(1 - p ** n)) This suggests @larryshamalama implementation is correct. I guess I was tripped by the cdf for discrete variables being P(X<=x) and not P(X<x) |
c885c32
to
4d2ac0d
Compare
The test "test_discrete_rv_multinary_transform_fails():" as it should. |
4d2ac0d
to
ff80553
Compare
Todo: |
a2ef7a7
to
69f1b32
Compare
Hey @ricardoV94 @larryshamalama . I have commited the changes discussed in the last week. Please give it a look as you find the time. |
4962e85
to
a0e811d
Compare
pymc/logprob/order.py
Outdated
|
||
|
||
measurable_ir_rewrites_db.register( | ||
"find_measurable_max_neg", | ||
find_measurable_max_neg, | ||
"basic", | ||
"min", | ||
"min_discrete", |
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.
"min_discrete", | |
"min", |
416d3ac
to
d97d0af
Compare
Co-authored-by: Dhruvanshu-Joshi <104030847+dhruvanshu-joshi@users.noreply.github.com>
d97d0af
to
829b63c
Compare
Great work @Dhruvanshu-Joshi |
What is this PR about?
This PR is the extension of PR 6790 and aims to provide a solution for implementing the Min operation for discrete distributions from order statistics to solve the issue 6350 and issue 6773.
CC: @larryshamalama @ricardoV94
Checklist
Major / Breaking Changes
New features
Bugfixes
Documentation
Maintenance
📚 Documentation preview 📚: https://pymc--6968.org.readthedocs.build/en/6968/