-
Notifications
You must be signed in to change notification settings - Fork 2
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
Gamma functions #67
Gamma functions #67
Conversation
(+ (reduce (fn [sum [i l]] (+ sum (/ l (+ x i)))) 0.0 c) | ||
0.99999999999999709182))) | ||
|
||
(defn inv-gamma-1pm1 |
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.
we could potentially ^:no-doc
this
(* (/ t x) (dec c)) | ||
(* x c)))))) | ||
|
||
(defn log-gamma-1p |
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.
we could potentially ^:no-doc
this
@@ -25,15 +27,15 @@ | |||
(with-comparator (within 1e-11) |
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.
we could eventually pull this out into it's own gamma-test
namespace.
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.
Couple of tiny nitpicks then let's MERGE
src/gen/distribution/math/gamma.cljc
Outdated
[3 14.136097974741746] | ||
[2 -59.59796035547549] | ||
[1 57.15623566586292]]] | ||
(+ (reduce (fn [sum [i l]] (+ sum (/ l (+ x i)))) 0.0 c) |
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.
slight style thing, you could start your reduction with this value instead of 0.0 and save a level of nesting
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.
yep, good catch—fixed.
src/gen/distribution/math/gamma.cljc
Outdated
0.166538611382291489501700795102105E+00 | ||
-0.420026350340952355290039348754298E-01 | ||
-0.655878071520253881077019515145390E+00]] | ||
(if (< t 0) |
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.
(neg? t)
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.
nice, done.
Fixed the nits, all checks passed—ready for merge. |
Boom, thanks for this, this is excellent work! |
Purpose
This PR adds native gamma function support with implementation references and sources (see the
math.gamma
namespace for details) and consolidates common math constants into a little namespace calledmath.utils
.Testing
All existing tests pass (
bb test:clj
,bb test:cljs
).Next steps
I would love a spot check and any feedback, happy to make changes. Otherwise it is my belief that this is good to go.