Implement icdf for Univariate distribution

[michaelraczycki]

What is this PR about?
Adding icdf functions for existing distributions
Creation of tests for added idcf functions

Major / Breaking Changes

• ...

New features

• icdf function for Uniform distritubtion

Bugfixes

• ...

Documentation

• ...

Maintenance

• extended test_continuous.py with corresponding tests

[michaelraczycki]

@ricardoV94 if that's what you had in mind I can continue adding them for other distributions, I wanted to create a PR before writing a lot to make sure we're on the same page

[ricardoV94]

Looks like a good start! Some issues mentioned in the comments below

[ricardoV94]

Looks good, left some comments below and don't miss the test :)

[codecov]

Codecov Report

Merging #6528 (63641a7) into main (67925df) will increase coverage by 0.00%.
The diff coverage is 93.33%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #6528   +/-   ##
=======================================
  Coverage   92.01%   92.01%           
=======================================
  Files          93       93           
  Lines       15737    15742    +5     
=======================================
+ Hits        14480    14485    +5     
  Misses       1257     1257           

Impacted Files	Coverage Δ
pymc/testing.py	91.79% <90.90%> (+0.01%)	⬆️
pymc/distributions/continuous.py	97.70% <100.00%> (+<0.01%)	⬆️

[michaelraczycki]

@ricardoV94 I've included tests in previous commits, following the lead from test_normal_icdf. Please take a look at it, as I might have missed something

[ricardoV94]

@ricardoV94 I've included tests in previous commits, following the lead from test_normal_icdf. Please take a look at it, as I might have missed something

Oh sorry I must have missed it, I was looking only at the last commit

[michaelraczycki]

I assume that I still need to add params like lower and upper for distribution tests that require them. I'm just not quite sure if it's possible to include conditional arguments in @pytest.mark.parametrize, or I should create another set of test parameters (try to group them up, so they can share the same test datasets)

[ricardoV94]

I assume that I still need to add params like lower and upper for distribution tests that require them. I'm just not quite sure if it's possible to include conditional arguments in @pytest.mark.parametrize, or I should create another set of test parameters (try to group them up, so they can share the same test datasets)

Each row in the parametrize should include one set of parameters that are jointly valid to test the icdf. Adding 2 or 3 rows should be enough.

[michaelraczycki]

I have a problem, Uniform distribution test requires some data transformation (uniform.icdf takes in value, lower and upper as parameters, and stats.uniform.ppf expects value, loc, scale= upper - lower) and I can't seem to figure it out. So far I have tried to extract the values given to uniform distribution and compute the needed values for uniform.ppf:

I must understand something wrong, because the test fails with this message:

[michaelraczycki]

I also tried to play with the idea of adding a new if-else to numpy_res assignment, in which I could maybe alter what goes to it:

problem in this approach would be that I can't come up with an idea to check if the passed variable is a reference to specific function. @ricardoV94 any suggestions on how to handle this?

[ricardoV94]

For the compatibility between PyMC and Scipy parametrizations you can wrap scipy icdf in a lambda function that does the parameter conversion (untested code and parametrization expression):

test_fn=lambda value, lower, upper: stats.uniform.icdf(value, lower, upper-lower)

[michaelraczycki]

@ricardoV94 , I've adjusted the uniform icdf to use the new value and parameter checks for icdf, I think we should merge it now so It can be used as a template for contributors with further icdf implementations

[ricardoV94]

@michaelraczycki the code still includes the untested icdf for the truncated normal, and it's using the old testing routine, not the new check_icdf introduced in 9ef9a14

[ricardoV94]

Looks good. Should be squashed merged