-
Notifications
You must be signed in to change notification settings - Fork 26
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
Fix zernike_eval notebook and Use integer division instead of gammaln() #1037
Changes from all commits
c55fc45
6b6c937
92726e4
c66da6b
4d9180b
606a930
0cbc083
d2aeb79
f8f1887
f95104d
ec5b9a5
7752a5f
347807e
c660cca
8a1d505
3eb59da
79388b7
f5766d0
1f1e3a9
28d4785
e6073af
bb8febd
7b164ef
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -457,8 +457,8 @@ def combination_permutation(m, n, equals=True): | |
def multinomial_coefficients(m, n): | ||
"""Number of ways to place n objects into m bins.""" | ||
k = combination_permutation(m, n) | ||
num = factorial(n) | ||
den = factorial(k).prod(axis=-1) | ||
num = factorial(n, exact=True) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what does There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here, we are using scipy s factorial function which is by default an approximation with gamma function. To be more accurate, we should use integer operations instead. Exact=true converts it to integer operation. https://docs.scipy.org/doc/scipy/reference/generated/scipy.special.factorial.html |
||
den = factorial(k, exact=True).prod(axis=-1) | ||
return num / den | ||
|
||
|
||
|
Large diffs are not rendered by default.
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.
there's no difference here right? Just the extra comments?
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.
Yes, there is no change. Additional lines confused git, I guess.