Skip to content

[Distributions] Add Binomial distribution & DiscretelyDistributed trait#3

Merged
forfudan merged 8 commits intomojomath:mainfrom
shivasankarka:distributions
Mar 7, 2026
Merged

[Distributions] Add Binomial distribution & DiscretelyDistributed trait#3
forfudan merged 8 commits intomojomath:mainfrom
shivasankarka:distributions

Conversation

@shivasankarka
Copy link
Copy Markdown
Member

Traits

DiscretelyDistributed trait has been implemented with basic method for the distributions. Any integral value must be of type Int for consistency.

Binomial

  • Binomial distribution has been implemented and it conforms to DiscretelyDistributed.
  • New tests have been added for Binomial distribution.
  • Updated imports & docstrings.

@shivasankarka
Copy link
Copy Markdown
Member Author

@forfudan I have currently made the field n to be of Int type even though is must always be positive. Should we add these checks in the constructor and raise instead? (I didn't see any other raising functions in the library so I assumed that we are trying to avoid raising functions).

@forfudan
Copy link
Copy Markdown
Member

forfudan commented Mar 7, 2026

@shivasankarka
Yes, at the moment I am trying to avoid raising. I think we can add it at a later time point to all distributions.

@forfudan forfudan self-requested a review March 7, 2026 10:02
Copy link
Copy Markdown
Member

@forfudan forfudan left a comment

Choose a reason for hiding this comment

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

Add checks for logpmf and cdf when k < 0.

For _log_binomial_coefficient(n: UInt, ...), change it to n: Int to be consistent with other cases. We may consider adding raises in future.

@forfudan forfudan merged commit 920dccc into mojomath:main Mar 7, 2026
1 check passed
@shivasankarka shivasankarka deleted the distributions branch March 7, 2026 15: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.

2 participants