Skip to content

Conversation

@jaksle
Copy link

@jaksle jaksle commented Dec 3, 2025

This is a very straightforward PR. It adds missing mgf, cgf, cf and direct affine transformations for the Cosine distribution. There are simple formulas for these, so there is no reason not to include them.

One thing which might require attention is if I protect from dividing by 0 correctly.

@codecov-commenter
Copy link

codecov-commenter commented Dec 3, 2025

Codecov Report

❌ Patch coverage is 82.75862% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.33%. Comparing base (e5d4fef) to head (f3d5850).

Files with missing lines Patch % Lines
src/univariate/continuous/cosine.jl 82.75% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2017      +/-   ##
==========================================
- Coverage   86.36%   86.33%   -0.04%     
==========================================
  Files         146      146              
  Lines        8788     8805      +17     
==========================================
+ Hits         7590     7602      +12     
- Misses       1198     1203       +5     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Member

@devmotion devmotion left a comment

Choose a reason for hiding this comment

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

Can you add tests for the new functionality?

@jaksle
Copy link
Author

jaksle commented Dec 5, 2025

I cleaned cf too. I also noticed the parameters are used once now, so their unpacking can be made even simpler.

I added testing against some known simple values. Some more are needed or is this enough?

My code was based on how the rest of cosine.jl looked like. When I saw your corrections the code above started looking suspicious for me and indeed, pdf, cdf and ccdf were type unstable. So I corrected them, though I'm not sure if my solution is optimal.

I'm also not sure about dependencies, but its' easy to change in any case.

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.

3 participants