Skip to content

test(distributions): add missing scipy logcdf and icdf tests for LogitNormal, Kumaraswamy, and ChiSquared - #8397

Closed
sankhyanreyansh wants to merge 2 commits into
pymc-devs:mainfrom
sankhyanreyansh:test-scipy-continuous-dists
Closed

test(distributions): add missing scipy logcdf and icdf tests for LogitNormal, Kumaraswamy, and ChiSquared#8397
sankhyanreyansh wants to merge 2 commits into
pymc-devs:mainfrom
sankhyanreyansh:test-scipy-continuous-dists

Conversation

@sankhyanreyansh

Copy link
Copy Markdown
Contributor

Description

This PR adds missing unit test coverage in tests/distributions/test_continuous.py under TestMatchesScipy to verify PyMC distribution functions against SciPy and analytical quantile formulations:

  • Added check_logcdf for LogitNormal against scipy.stats.norm.logcdf(scipy.special.logit(x), mu, sigma).
  • Added check_icdf for Kumaraswamy against its analytical quantile function $(1 - (1 - q)^{1/b})^{1/a}$.
  • Added check_icdf for ChiSquared against scipy.stats.chi2.ppf.

Related Issue

Checklist

Type of change

  • New feature / enhancement
  • Bug fix
  • Documentation
  • Maintenance
  • Other (please specify):

@codecov

codecov Bot commented Aug 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.87%. Comparing base (47bdf54) to head (fdd6c1e).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #8397   +/-   ##
=======================================
  Coverage   91.87%   91.87%           
=======================================
  Files         128      128           
  Lines       21256    21256           
=======================================
  Hits        19530    19530           
  Misses       1726     1726           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ricardoV94 ricardoV94 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This PR is 2 misses and maybe 1 hit. Please don't open PRs in search of a motivation.

Comment thread tests/distributions/test_continuous.py Outdated
lambda value, nu: st.chi2.logcdf(value, df=nu),
)

def test_chisquared_icdf(self):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is just a gamma parametrization, no point in spending CI cycles retesting it

Comment thread tests/distributions/test_continuous.py Outdated
),
decimal=select_by_precision(float64=6, float32=1),
)
check_logcdf(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is just a customdist with automatically derived logcdf, no point in spending cycles retesting it.

@read-the-docs-community

read-the-docs-community Bot commented Aug 13, 2026

Copy link
Copy Markdown

Documentation build overview

📚 pymc | 🛠️ Build #34049993 | 📁 Comparing fdd6c1e against latest (47bdf54)

  🔍 Preview build  

1 file changed
± glossary.html

@sankhyanreyansh

Copy link
Copy Markdown
Contributor Author

Hi @ricardoV94,
I apologize for taking up your time with redundant tests and for opening a PR without thoroughly checking the underlying codebase implementation first. I've pushed an update removing both the ChiSquared and LogitNormal tests. I kept only the Kumaraswamy ICDF test, but if you prefer I close this PR entirely or remove that as well, please let me know and I will do so.
I really appreciate the guidance and the work you do maintaining PyMC. Moving forward, I'll make sure to anchor my contributions to active open issues and inspect the core implementations far more carefully beforehand.

{"a": Rplus, "b": Rplus},
scipy_log_cdf,
)
check_icdf(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

actually we have the check_selfconsistency_icdf. The reason we didn't have this tset was scipy doesn't have it so we don't have a great "ground truth", that's better than what check_selfconsistency_icdf already does

@ricardoV94 ricardoV94 closed this Aug 14, 2026
@sankhyanreyansh
sankhyanreyansh deleted the test-scipy-continuous-dists branch August 14, 2026 05:45
@ricardoV94

Copy link
Copy Markdown
Member

No worries @sankhyanreyansh. Apologies for my rudeness

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants