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

[Feature Request] **Important** Rework Distribution<'a,'b> interface #223

Closed
Freymaurer opened this issue Jun 24, 2022 · 1 comment
Closed

Comments

@Freymaurer
Copy link
Contributor

Is your feature request related to a problem? Please describe.
At the moment Distributions.Discrete.Hypergeometric.CDF has input parameter k: float. k is the number of success events and must be a non negative integer. It is missleading to expect a float at this position, only to (floor k |> int) it later.

This issue is based on the Distribution<'a,'b> interface, in which both Mean (which must be float) as well as CDF use type 'a. With the current interface it is not possible to use a CDF of type int -> float for discrete distributions.

type Distribution<'a, 'b> =
abstract Mean : 'a
abstract StandardDeviation : 'a
abstract Variance : 'a
//abstract CoVariance : 'b
abstract Sample : unit -> 'b
abstract PDF : 'b -> float
abstract CDF : 'a -> float
//abstract PDFLn : 'a -> float
//abstract CDFLn : 'a -> float

⚠️ In addition, the current implementation is insufficient for a good usability. The general usage, which is also shown in the docs, does not return any sort of code documentation to the user. It is impossible to know which kind of CDF is implemented, especially as the FSharp.Stats CDF functions do not follow the same pattern. (Bernoulli implements P(X>=k), whereas hypergeometric implements P(X<=k)).

image

Describe the solution you'd like
Rework the Distribution<'a,'b> handling. I am not even sure if using a interface is the correct way, as it currently prevents correct code documentation. But it is probably best to split the type into discrete and continuous.

@bvenn
Copy link
Member

bvenn commented Jan 31, 2023

closed by d9e5be5

@bvenn bvenn closed this as completed Jan 31, 2023
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

No branches or pull requests

2 participants