Skip to content
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

Add faster Asymmetric Laplace #364

Merged
merged 8 commits into from
Mar 20, 2024
Merged

Conversation

rohanbabbar04
Copy link
Contributor

Description

  • Add class AsymmetricLaplace with all methods.
  • Update test in test_scipy.py.

Checklist

  • Code style is correct (follows pylint and black guidelines)
  • Includes new or updated tests to cover the new feature
  • New features are properly documented (with an example if appropriate)

@codecov-commenter
Copy link

codecov-commenter commented Mar 19, 2024

Codecov Report

Attention: Patch coverage is 76.22951% with 29 lines in your changes are missing coverage. Please review.

Project coverage is 82.42%. Comparing base (c62e23b) to head (ad2fc89).
Report is 1 commits behind head on main.

Files Patch % Lines
preliz/distributions/asymmetric_laplace.py 76.03% 29 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #364      +/-   ##
==========================================
- Coverage   82.71%   82.42%   -0.30%     
==========================================
  Files          55       56       +1     
  Lines        5162     5235      +73     
==========================================
+ Hits         4270     4315      +45     
- Misses        892      920      +28     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rohanbabbar04
Copy link
Contributor Author

rohanbabbar04 commented Mar 19, 2024

  • I was getting an OverFlow Error for the line which I commented out having ZeroInflatedBinomial.

For the rvs function, I implemented https://en.m.wikipedia.org/wiki/Asymmetric_Laplace_distribution#Generating_asymmetric_Laplace_variates, but the results do not match...
If you have time can you also try it and let me know, also it works only for a specific range only which is (-κ, 1/κ).

Currently, I have set it to the self.ppf(which is the other obvious alternative).

@aloctavodia
Copy link
Contributor

aloctavodia commented Mar 19, 2024

Can you open an issue for the overflow of zeroinflatedbinomial?
Can you send me the code for the rvs that do not match (here or by e-mail)?

You are right using the inverse cdf method is a general method to obtain random samples. One reason to have custom samples is speed. But maybe here it will be enough.

@rohanbabbar04
Copy link
Contributor Author

Can you open an issue for the overflow of zeroinflatedbinomial? Can you send me the code for the rvs that do not match (here or by e-mail)?

You are right using the inverse cdf method is a general method to obtain random samples. One reason to have custom samples is speed. But maybe here it will be enough.

  • Sure, I will open an issue for the OverflowError.
  • If using self.ppf is enough to handle the rvs you can review the PR, I will send you the code for the rvs mentioned in the link.

@aloctavodia
Copy link
Contributor

I will wait for the rvs code before reviewing

@rohanbabbar04
Copy link
Contributor Author

I will wait for the rvs code before reviewing

Sure, currently a little busy will provide you with the code tomorrow.

preliz/distributions/asymmetric_laplace.py Outdated Show resolved Hide resolved
preliz/distributions/asymmetric_laplace.py Outdated Show resolved Hide resolved
preliz/distributions/asymmetric_laplace.py Outdated Show resolved Hide resolved
@aloctavodia aloctavodia merged commit bdd33d2 into arviz-devs:main Mar 20, 2024
4 checks passed
@rohanbabbar04 rohanbabbar04 deleted the asy_laplace branch March 20, 2024 17:08
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.

None yet

3 participants