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

added beta mode tests #323

Merged
merged 11 commits into from
Feb 21, 2024
Merged

Conversation

nishant42491
Copy link
Contributor

@nishant42491 nishant42491 commented Feb 18, 2024

Description

This PR adds tests for beta_mode and makes changes to the beta call function interface.
resolves #325, resolves #320

Checklist

  • Code style is correct (follows pylint and black guidelines)
  • Includes new or updated tests to cover the new feature

@nishant42491 nishant42491 mentioned this pull request Feb 18, 2024
@nishant42491
Copy link
Contributor Author

@aloctavodia is this what you were broadly looking for?

@aloctavodia
Copy link
Contributor

The beta distribution is defined on the 0-1 interval. So for any beta distribution 100% of the mass will necessarily be inside that interval. In other words it is not posible to define a beta distribution with a mass less than 1 if the lower bound is 0 and the upper bound is 1.

@aloctavodia
Copy link
Contributor

Hi @nishant42491 Let me know if you need help.

@nishant42491
Copy link
Contributor Author

Hi @nishant42491 Let me know if you need help.

hey there, I was a bit confused about how to compare the values of the fitted distribution. maybe use the distribution parameters which we receive from maxent let me know what you think.

Also, I have included a minor fix to the mode value error included in the beta_mode function. The function does not converge when mode = lower bound or mode = upper bound hence a strict inequality is needed for that case.

@nishant42491
Copy link
Contributor Author

The beta distribution is defined on the 0-1 interval. So for any beta distribution 100% of the mass will necessarily be inside that interval. In other words it is not posible to define a beta distribution with a mass less than 1 if the lower bound is 0 and the upper bound is 1.

i have tried to fix this in my latest commit.

preliz/tests/test_beta_mode.py Outdated Show resolved Hide resolved
preliz/tests/test_beta_mode.py Outdated Show resolved Hide resolved
@nishant42491
Copy link
Contributor Author

yea, you are right the dist parameters should be symmetric. I have added a fix for that.

@aloctavodia
Copy link
Contributor

Almost there, you need to run black.

You can install https://pre-commit.com/ and it will alert you of errors with linters before committing.

@nishant42491
Copy link
Contributor Author

yep, black seems to have fixed all the linting issues

@codecov-commenter
Copy link

codecov-commenter commented Feb 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (c82028a) 83.31% compared to head (cda9df4) 84.04%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #323      +/-   ##
==========================================
+ Coverage   83.31%   84.04%   +0.72%     
==========================================
  Files          36       37       +1     
  Lines        4244     4262      +18     
==========================================
+ Hits         3536     3582      +46     
+ Misses        708      680      -28     

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

@nishant42491
Copy link
Contributor Author

I have also set the default plot keyword to True in Beta_Mode which should fix issue #325. I have linked the issue through the description of this PR.

preliz/tests/test_beta_mode.py Outdated Show resolved Hide resolved
@aloctavodia aloctavodia merged commit 4404ec3 into arviz-devs:main Feb 21, 2024
4 checks passed
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.

beta_mode: plot should default to True Add test beta_mode
3 participants