-
Notifications
You must be signed in to change notification settings - Fork 23
Conversation
distmv/normal.go
Outdated
// Quantile returns the multi-dimensional inverse cumulative distribution function. | ||
// len(x) must equal len(p), and if x is non-nil, len(x) must also equal len(p). | ||
// If x is nil, a new slice will be allocated and returned, otherwise the quantile | ||
// will be stored in-place into x. All of the values of p must be between 0 and 1, |
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.
inclusive
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.
Done.
distmv/normal.go
Outdated
@@ -246,6 +246,29 @@ func (n *Normal) Prob(x []float64) float64 { | |||
return math.Exp(n.LogProb(x)) | |||
} | |||
|
|||
// Quantile returns the multi-dimensional inverse cumulative distribution function. | |||
// len(x) must equal len(p), and if x is non-nil, len(x) must also equal len(p). |
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.
I don't understand this.
Also, at the moment there is no explicit length check for x
when x != nil
; the panic is potentially a runtime bounds check if len(x) < len(p)
, otherwise a panic in TransformNormal
.
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.
Added the explicit check for size.
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.
I still don't understand "len(x) must equal len(p), and if x is non-nil, len(x) must also equal len(p)." the first "len(x)" is presumably supposed to be something else, no?
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.
Sorry, the comment was non-sensical. Fixed.
distmv/normal.go
Outdated
func (n *Normal) Quantile(x, p []float64) []float64 { | ||
dim := n.Dim() | ||
if len(p) != dim { | ||
panic(badSizeMismatch) |
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.
badInputLength
?
(I'm not sure that badSizeMismatch
is a good label anyway - what does it mean? A really bad size mismatch or a size mismatch is not actually a size mismatch).
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.
Done.
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.
What is badSizeMismatch
supposed to be used for? I think we should bring these constants together - having them spread around in separate files the way they are leads to this kind of problem.
ping |
Thanks |
802df93
to
ef6396f
Compare
No description provided.