Weibull-distribution #114
Weibull-distribution #114
Conversation
[x] implementation [x] tests [x] documentation
pavpanchekha
left a comment
pavpanchekha
left a comment
There was a problem hiding this comment.
I am not really knowledgeable Weibull distributions specifically, but I did spot check the code and it looks good, and it's documented and tested, so in general I'm willing to merge. Any specific comments?
| (let ([sampler (make-weibull-sampler k)]) | ||
| (values (make-weibull-pdf k) | ||
| (make-weibull-cdf k) | ||
| (make-weibull-inverse-cdf k) | ||
| (case-lambda: | ||
| [() (unsafe-flvector-ref (sampler 1) 0)] | ||
| [([n : Integer]) (flvector->list (sampler n))]))) | ||
| (let ([sampler (make-weibull-sampler k d s)]) | ||
| (values (make-weibull-pdf k d s) | ||
| (make-weibull-cdf k d s) | ||
| (make-weibull-inverse-cdf k d s) | ||
| (case-lambda: | ||
| [() (unsafe-flvector-ref (sampler 1) 0)] | ||
| [([n : Integer]) (flvector->list (sampler n))]))))) |
There was a problem hiding this comment.
Is there a reason to have the if statement on std? here? If I'm reading the code correctly, all of the make-weibull-pdf and similar functions can take three arguments and branch internally instead? Not a huge deal either way.
There was a problem hiding this comment.
Indeed, I can move the (and (= d 0) (= s 1)) test to the implementation side to avoid the extra wrapping when there is a std? call.
I think it is better to do it only once, here. If somebody would use the internal functions it is their responsibility to not use the 3 value version when not needed.
But I could change the 1 argument version to avoid the extra wrapping call and immediately use make-std-weibull-… functions.
Please let me know your preference.
There was a problem hiding this comment.
Nah, let's merge as is. I just wanted to understand what's up.
|
Thanks for looking into this. Among other things the Weibull distribution is used for:
Error: |
2 participants
too model all my failures
(or on brighter days - when to go surfing)
edit:
extra testfiles - weibulldist.zip