Skip to content

Make logprob inference for binary ops independent of order of inputs #6682

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

Merged

Conversation

shreyas3156
Copy link
Contributor

@shreyas3156 shreyas3156 commented Apr 20, 2023

What is this PR about?
This PR makes the logprob derivation of binary comparison ops (<=, >=, <, >) independent of the order of the inputs. e.g. pt.le(x, 0.4) and pt.le(0.4, x) would have the same logp.

Addresses #6633.

Checklist


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

@codecov
Copy link

codecov bot commented Apr 20, 2023

Codecov Report

Merging #6682 (3009cde) into main (9b712bf) will decrease coverage by 0.02%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #6682      +/-   ##
==========================================
- Coverage   92.01%   91.99%   -0.02%     
==========================================
  Files          95       95              
  Lines       16000    16016      +16     
==========================================
+ Hits        14722    14734      +12     
- Misses       1278     1282       +4     
Impacted Files Coverage Δ
pymc/logprob/binary.py 96.96% <100.00%> (-1.04%) ⬇️

... and 1 file with indirect coverage changes

@shreyas3156 shreyas3156 force-pushed the binary-input-order-agnostic-6633 branch from 907bab0 to c0a45d4 Compare April 20, 2023 17:56
@shreyas3156
Copy link
Contributor Author

@ricardoV94 Once this is merged, I can create a PR for the not-operation logprob inference.

@ricardoV94 ricardoV94 merged commit 55d915c into pymc-devs:main Apr 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants