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

Enable that location and scale parameters might be set or not used. #597

Merged

Conversation

veni-vidi-vici-dormivi
Copy link
Collaborator

Some distributions do not use loc and scale. While all distributions have loc as parameter, for some it might make sense to set it to 0 like the beta distribution or even just if one has calculated residuals for their data and knows the loc is 0. Thus, I implemented an option to set the location in the expression to a certain value (note because of #525 it can only be ints atm). Moreover, the discrete distributions tend to not have a scale parameter so I made it optional.

  • Tests added
  • Fully documented, including CHANGELOG.rst

@veni-vidi-vici-dormivi
Copy link
Collaborator Author

Test for discrete distributions that actually omit the scale parameter follow in another PR as they also need some other changes.

Copy link

codecov bot commented Jan 23, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.13%. Comparing base (4ef7a1f) to head (f252fda).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #597      +/-   ##
==========================================
+ Coverage   80.12%   80.13%   +0.01%     
==========================================
  Files          49       49              
  Lines        3064     3066       +2     
==========================================
+ Hits         2455     2457       +2     
  Misses        609      609              
Flag Coverage Δ
unittests 80.13% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@veni-vidi-vici-dormivi veni-vidi-vici-dormivi changed the title Enable that location and sale parameters might be set or not used. Enable that location and scale parameters might be set or not used. Jan 23, 2025
@veni-vidi-vici-dormivi veni-vidi-vici-dormivi requested review from yquilcaille and removed request for mathause January 23, 2025 14:49
Copy link
Member

@mathause mathause left a comment

Choose a reason for hiding this comment

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

Looks good.

Copy link
Collaborator

@yquilcaille yquilcaille left a comment

Choose a reason for hiding this comment

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

I think that the modifications done make sense. I simply suggest to add for the location (line 1039) the logic used for the scale with the try / except (line 1060-1062).

Co-authored-by: Mathias Hauser <mathause@users.noreply.github.com>
@veni-vidi-vici-dormivi veni-vidi-vici-dormivi merged commit 6a6b2f1 into MESMER-group:main Jan 24, 2025
11 checks passed
@veni-vidi-vici-dormivi veni-vidi-vici-dormivi deleted the scale_opt branch January 24, 2025 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants