feat: add c implementation for stats/base/dists/poisson/logpmf
#4954
Conversation
|
|
||
| #### stdlib_base_dists_poisson_logpmf( x, lambda ) | ||
|
|
||
| Evaluates the natural logarithm of the probability mass function (PMF) for a Poisson distribution with mean parameter `lambda` at a value `x`. |
There was a problem hiding this comment.
| Evaluates the natural logarithm of the probability mass function (PMF) for a Poisson distribution with mean parameter `lambda` at a value `x`. | |
| Evaluates the natural logarithm of the probability mass function (PMF) for a Poisson distribution with mean parameter `lambda`. |
There was a problem hiding this comment.
It should match the JSDoc API call mentioned above
There was a problem hiding this comment.
I believe you should change this as well for main.c & include.h
|
|
||
|
|
||
| /** | ||
| * Evaluates the quantile function for a Poisson distribution with mean parameter `lambda` at a probability `p`. |
There was a problem hiding this comment.
I guess you copied the incorrect call from poisson/quantile.
There was a problem hiding this comment.
Okay let's see
| #endif | ||
|
|
||
| /** | ||
| * Returns the logpmf of a Poisson distribution. |
There was a problem hiding this comment.
My mistake
|
@saurabhraghuvanshii |
lib/node_modules/@stdlib/stats/base/dists/poisson/logpmf/benchmark/benchmark.js
Show resolved
Hide resolved
| b.tic(); | ||
| for ( i = 0; i < b.iterations; i++ ) { | ||
| x = ceil( randu()*50.0 ); | ||
| y = mypmf( x ); |
There was a problem hiding this comment.
| y = mypmf( x ); | |
| y = mypmf( x[ i % len ] ); |
There was a problem hiding this comment.
as you want to implement the discreteUniform logic that you created for var x for your function mypmf() as well right
But I refer issue for title |
| "@stdlib/math/base/special/factorialln", | ||
| "@stdlib/math/base/special/ceil", | ||
| "@stdlib/math/base/special/ln", | ||
| "@stdlib/math/base/assert/is-nan" |
There was a problem hiding this comment.
| "@stdlib/math/base/assert/is-nan" | |
| "@stdlib/math/base/assert/is-nan", | |
| "@stdlib/constants/float64/pinf", | |
| "@stdlib/constants/float64/ninf" |
There was a problem hiding this comment.
do the same for other builds as well!
Ok, but while creating a PR for that issue we refer the package name from the root package in which the package is added. |
saurabhraghuvanshii
changed the title
@stdlib/stats/base/dists/poisson/logpmfstats/base/dists/poisson/logpmf
Coverage ReportNo coverage information available. |
|
done everything as your suggestion. |
| var i; | ||
|
|
||
| lambda = 10.0; | ||
| lambda = 100; |
There was a problem hiding this comment.
please take a look at the suggested change properly.
you require to set a value for var len and not tamper the original set val of lambda
There was a problem hiding this comment.
aaaah man sorry
| * Evaluates the quantile function for a Poisson distribution with mean parameter `lambda`. | ||
| * | ||
| * @param x input value | ||
| * @param lambda mean parameter |
There was a problem hiding this comment.
this should be stacked one above the another
There was a problem hiding this comment.
in local it is align but in commit it doesn't show
There was a problem hiding this comment.
it's because they are tab indented which is incorrect there should be spaces.
There was a problem hiding this comment.
how I can do this
There was a problem hiding this comment.
do one thing remove the entire C doxygen and write it again this time using spaces.
lib/node_modules/@stdlib/stats/base/dists/poisson/logpmf/README.md
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/stats/base/dists/poisson/logpmf/manifest.json
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/stats/base/dists/poisson/logpmf/manifest.json
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/stats/base/dists/poisson/logpmf/manifest.json
Outdated
Show resolved
Hide resolved
| delta = abs( y - expected[ i ] ); | ||
| tol = 80.0 * EPS * abs( expected[ i ] ); | ||
| t.ok( delta <= tol, 'within tolerance. x: '+x[ i ]+'. lambda: '+lambda[i]+'. y: '+y+'. E: '+expected[ i ]+'. Δ: '+delta+'. tol: '+tol+'.' ); |
There was a problem hiding this comment.
As suggested by the maintainers.
Should check whether the non-exact check is necessary. There are no calculations that would suggest it's needed. If also present in test file, would be good to remove from that as well.
There was a problem hiding this comment.
it not in test.js it is in test.logpmf.js should I do changes in that file.
There was a problem hiding this comment.
yea that would be desirable.
|
@saurabhraghuvanshii Thanks for the PR. Left a few comments other than that everything else is in pretty good shape. |
saurabhraghuvanshii
force-pushed
the
poisson-logpmf
branch
from
576496d to
d9c08b0
Compare
|
@Neerajpathak07 lost my all commit try recover but didn't joning to create a new pr |
|
@Neerajpathak07 you can check in new pr #5194 if any changes need i do . thanks |
3 participants
type: pre_commit_static_analysis_report
description: Results of running static analysis checks when committing changes. report:
Resolves #3788
Description
This pull request:
@stdlib/stats/base/dists/poisson/logpmfRelated Issues
This pull request:
@stdlib/stats/base/dists/poisson/logpmf#3788Questions
No.
Other
No.
Checklist
@stdlib-js/reviewers