Add asymetric laplace distribution

[twiecki]

There is an implementation here we could port quite easily: https://github.com/epidemics/COVIDNPIs/blob/manuscript/epimodel/pymc3_distributions/asymmetric_laplace.py

[chandan5362]

Hey @twiecki , could you please explain the issue a bit more. I would love to work on this issue.

[twiecki]

@chandan5362 That's great. As you can see in the link, the distribution is already implemented. So you would copy & paste that into distributions/continuous.py, make sure the doc string matches the ones we already have (e.g. creates a plot) and add unittests.

[chandan5362]

@twiecki Thanks for the help. I am little confused here. where should I add unittests ?. whether should I create separate file or I will have to add it to the existing one.

[ricardoV94]

@chandan5362 you should add your unit test in the existing files with unit tests for the other distributions:
https://www.github.com/pymc-devs/pymc3/tree/master/pymc3%2Ftests%2Ftest_distributions.py

You can also have a look and check what kind of tests are there for the existing distributions.

[CloudChaoszero]

If there are no assignees to this sometime soon, I am happy to submit a PR in the near future.

• Feel free to @ me.

[twiecki]

@chandan5362 Are you actively working on this?
@CloudChaoszero I would say you can start tomorrow if you we don't hear.

[chandan5362]

@twiecki @CloudChaoszero yeah, I am still working on it and I will make a PR tomorrow or day after tomorrow.

[CloudChaoszero]

@twiecki @CloudChaoszero yeah, I am still working on it and I will make a PR tomorrow or day after tomorrow.

Oh hey, no worries @chandan5362, just checking to see if it was officially active. haha
(Take your time, it's the holidays 😄)

[chandan5362]

hey @twiecki , as https://github.com/epidemics/COVIDNPIs/blob/manuscript/epimodel/pymc3_distributions/asymmetric_laplace.py contains only logpdf and all other instances of test functions in test_distribution.py contains both logpdf and logcdf. So, would it be alright if I add unit test only for logpdf. Since I am new to this repo and still understanding the codebase, a little help in writing the unit test would be much helpful 😐.
Thanks in advance.

[twiecki]

@chandan5362 It's fine to not have a logcdf for this one. You can just copy a test for another distribution in test_distribution.py where we compare against the scipy implementation, and use https://scipy.github.io/devdocs/generated/scipy.stats.laplace_asymmetric.html. Does that make sense?

[chandan5362]

yeah , sounds cool.
Thanks a lot @twiecki

[chandan5362]

hey @twiecki , would you please help me here a little ?
As i can see that the current version of SciPy that we are using is 1.5.4 and that does not support laplace_assymetric. and the latest version of SciPy is not released yet. So, I am not able to import it for the unit test. How should I deal with it ?

[ricardoV94]

You could just copy their logpdf function from the source code: https://github.com/scipy/scipy/blob/74875a8e60ba1117dc98743b3297234f5ea6cf9a/scipy/stats/_continuous_distns.py#L4143-L4147

A similar thing is done for the discrete_weibull right now, since it is not available on scipy:

https://github.com/pymc-devs/pymc3/blob/3cfee77fda041483e86d9c61fa1d1ada9380cdb3/pymc3/tests/test_distributions.py#L383-L389

https://github.com/pymc-devs/pymc3/blob/3cfee77fda041483e86d9c61fa1d1ada9380cdb3/pymc3/tests/test_distributions.py#L1044-L1050

[chandan5362]

sounds cool.
thanks @ricardoV94

[Sayam753]

Closed in #4392