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

Derive logprob of >= and <= operations #6680

Merged

Conversation

shreyas3156
Copy link
Contributor

@shreyas3156 shreyas3156 commented Apr 19, 2023

What is this PR about?
This PR implements the logprob inference for binary comparison Ops >= and <= in continuation to #6662.
The derivation for discrete distributions has been implemented separately from continuous to club together the similarities in logprob of <= and > ops.

Addresses #6633.

Checklist

New features

  • Logprob derivation for GE and LE ops.

📚 Documentation preview 📚: https://pymc--6680.org.readthedocs.build/en/6680/

@codecov
Copy link

codecov bot commented Apr 19, 2023

Codecov Report

Merging #6680 (40de354) into main (9b712bf) will decrease coverage by 4.33%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #6680      +/-   ##
==========================================
- Coverage   92.01%   87.68%   -4.33%     
==========================================
  Files          95       95              
  Lines       16000    16003       +3     
==========================================
- Hits        14722    14032     -690     
- Misses       1278     1971     +693     
Impacted Files Coverage Δ
pymc/logprob/binary.py 98.11% <100.00%> (+0.11%) ⬆️

... and 8 files with indirect coverage changes

Copy link
Member

@ricardoV94 ricardoV94 left a comment

Choose a reason for hiding this comment

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

Awesome 😎

Next step would be rewrites to allow dists to be on the right side of constants?

And maybe less important but does not operation make sense as a rewrite as well? not(gt(x, const), True) -> (gt(x, const), not(True))?

@shreyas3156
Copy link
Contributor Author

Next step would be rewrites to allow dists to be on the right side of constants?

Yes, I'll use the logic in transforms.py to take into account the measurable input index for such cases.

And maybe less important but does not operation make sense as a rewrite as well? not(gt(x, const), True) -> (gt(x, const), not(True))?

This may be helpful if we are to tackle nested switch statements with overlapping intervals, as in the example mentioned in #6354.

@ricardoV94 ricardoV94 merged commit 61be336 into pymc-devs:main Apr 19, 2023
@ricardoV94 ricardoV94 changed the title Add logprob derivation for >= and <= operations Add logprob derivation for >= and <= operations Apr 26, 2023
@ricardoV94 ricardoV94 changed the title Add logprob derivation for >= and <= operations Derive logprob of >= and <= operations Apr 26, 2023
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.

2 participants