Skip to content

Fix logcdf and icdf derivations for non-monotonically increasing transformations #6850

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
merged 4 commits into from
Aug 13, 2023

Conversation

ricardoV94
Copy link
Member

@ricardoV94 ricardoV94 commented Aug 8, 2023

#6597 was only valid for strictly monotonically increasing transformations.

This PR fixes the derivations for monotonically decreasing transformations and adds a special case for logcdf of negative powers for distributions that allow negative values.

Also fixes the derivations for cosh transformed variables as that is not injective (CC @LukeLB)

Refactored a bit the test module with Test classes. It was growing a tad too unmanageable.
The first two commits are just refactoring and bugfixes come after


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

@ricardoV94 ricardoV94 force-pushed the fix_logcdf_reflections branch 2 times, most recently from 3629fb1 to c4f3b37 Compare August 8, 2023 09:05
@codecov
Copy link

codecov bot commented Aug 8, 2023

Codecov Report

Merging #6850 (bafe249) into main (ccad4c8) will decrease coverage by 0.02%.
Report is 2 commits behind head on main.
The diff coverage is 91.17%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #6850      +/-   ##
==========================================
- Coverage   92.05%   92.03%   -0.02%     
==========================================
  Files          96       96              
  Lines       16369    16398      +29     
==========================================
+ Hits        15068    15092      +24     
- Misses       1301     1306       +5     
Files Changed Coverage Δ
pymc/logprob/transforms.py 94.42% <90.90%> (-0.27%) ⬇️
pymc/testing.py 91.46% <100.00%> (ø)

... and 2 files with indirect coverage changes

@ricardoV94 ricardoV94 force-pushed the fix_logcdf_reflections branch from c4f3b37 to bafe249 Compare August 10, 2023 09:01
Copy link
Member

@larryshamalama larryshamalama left a comment

Choose a reason for hiding this comment

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

Apologies for the delay... Looks great @ricardoV94 :) No major questions from me this time. Admittedly, I didn't look too carefully at the tests, I have full confidence in them 😅

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