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

Adds sample default values for Binomial & Categorical random variables #35

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

saresend
Copy link

What

This PR adds some default implementations for categorical and binomial distributions. Further, it also adds an owned initializer to the categorical random variable so that it is slightly more ergonomic to write code like the following:

let categorical = distribution::Categorical::new(vec![....]);

@IvanUkhov
Copy link
Member

I am really uncomfortable adding that kind of defaults. Sorry.

But what is your use case? What do you need Default to be implemented?

@saresend
Copy link
Author

My use case is that I'd like to wrap collections of random variables that model some degree of dependence on each other. While using certain distributions these collection types implement Default, but its a bit inconsistent in that changing some of the internal distributions can break that assumption. Of course I could wrap these things myself, but it felt a bit cleaner to let all distributions implement a default type that provides some non-trivial instance, so that it everything becomes derivable, rather than have to handle things manually.

@saresend
Copy link
Author

But yeah, I understand your concern - I'm not sure if there'd be another trait that denotes less of a "standard" instance, and rather some instance that's convenient to construct?

@IvanUkhov
Copy link
Member

My use case is that I'd like to wrap collections of random variables that model some degree of dependence on each other. While using certain distributions these collection types implement Default, but its a bit inconsistent in that changing some of the internal distributions can break that assumption.

Why is it necessary for your higher-level abstraction to have the underlying distribution implement Default? Perhaps it is imposed artificially and is not actually needed? The distributions are probably stored in some Vec, and Vec implements Default regardless of what is inside. Or you could implement Default conditionally:

impl Default for MyRandomVector<T>
where
    T: Distribution + Default,
{
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants