-
Notifications
You must be signed in to change notification settings - Fork 231
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
Probability package updates #3478
Conversation
245dee4
to
bafb3e8
Compare
Also see #1229. Is there a standard data type for distributions? |
There is in the Probability package -- see https://macaulay2.com/doc/Macaulay2/share/doc/Macaulay2/Probability/html/___Probability__Distribution.html |
I know it'll be a lot of work, but I think it would be useful down the road if I could pass a distribution as an option to random. |
That's already possible in the I suppose you're suggesting things like random ring elements and random matrices? That would be pretty easy to implement in the package, I think, by adding more methods like |
I guess I want everything to be compatible, so random(QQ, ...) can be changed to random(QQ^n, ...) or random(QQ^m, QQ^n, ...), and further random(S = QQ[x], d, ...) or random(S^n, S^{...}, ...) would also pull coefficients from the same distributions. I wouldn't be opposed to preloading the Probability package if necessary. More specifically about this PR, why did you choose normal distribution over uniform distribution? |
Good ideas! I'm going to remove most of the engine stuff from this PR (keeping the I went with normal because of the central limit theorem -- it shows up so frequently in applications! And also we don't have to worry about the support -- what interval would we use for a uniform distribution? But if we were to change the default for |
6197af0
to
f598278
Compare
I've simplified this to now just add |
I think since |
That makes sense! I'll work on a revised |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not necessarily in this PR, but it would be good to rename rawRandomRR to rawRandomRRuniform for clarity.
Currently, the probability distribution of
random(QQ)
is kind of unusual. If X and Y both have the discrete uniform distribution on {1,...,h} for some positive integer h, then we get a sample from the distribution of X/Y. This maybe isn't great. For example, P(X/Y = 1) = 1/h. Indeed:Instead, we propose sampling from the standard normal distribution and rounding the results to the nearest rational number with denominator bounded by h (using the Farey sequence). I think I mentioned this idea recently in some discussion, but I can't find it. After the change:
This PR also has a few other related changes:
RR
from the standard normal distribution and use it in theProbability
package.Probability
package documentation.This possibly closes #999, although that suggests using a uniform distribution.