-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
feat: add implementation of halfnormal/mean
#9613
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
base: develop
Are you sure you want to change the base?
feat: add implementation of halfnormal/mean
#9613
Conversation
…20420/stdlib into shubham2204-halfnornmal
Coverage Report
The above coverage report was generated for the changes in this PR. |
Signed-off-by: Shubham <[email protected]>
Signed-off-by: Shubham <[email protected]>
…20420/stdlib into shubham2204-halfnornmal
Signed-off-by: Shubham <[email protected]>
Signed-off-by: Shubham <[email protected]>
…20420/stdlib into shubham2204-halfnornmal
Planeshifter
left a comment
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.
Flagged a bunch of issues.
lib/node_modules/@stdlib/stats/base/dists/halfnormal/mean/src/addon.c
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/stats/base/dists/halfnormal/mean/src/main.c
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/stats/base/dists/halfnormal/mean/manifest.json
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/stats/base/dists/halfnormal/mean/benchmark/benchmark.native.js
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/stats/base/dists/halfnormal/mean/package.json
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/stats/base/dists/halfnormal/mean/docs/repl.txt
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/stats/base/dists/halfnormal/mean/docs/types/index.d.ts
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/stats/base/dists/halfnormal/mean/docs/types/index.d.ts
Outdated
Show resolved
Hide resolved
|
thankyou for your reviews sir @Planeshifter , I have applied the requested changes. |
halfnormal/meanhalfnormal/mean
Planeshifter
left a comment
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.
Thanks for your continued efforts! Should be ready to land soon.
lib/node_modules/@stdlib/stats/base/dists/halfnormal/mean/test/test.js
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/stats/base/dists/halfnormal/mean/test/test.js
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/stats/base/dists/halfnormal/mean/test/test.js
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/stats/base/dists/halfnormal/mean/test/test.native.js
Show resolved
Hide resolved
lib/node_modules/@stdlib/stats/base/dists/halfnormal/mean/src/main.c
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/stats/base/dists/halfnormal/mean/manifest.json
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/stats/base/dists/halfnormal/mean/test/test.js
Outdated
Show resolved
Hide resolved
|
I have applied the requested suggestion sir. |
Planeshifter
left a comment
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.
A few last comments.
| "expected", | ||
| "halfnormal", | ||
| "univariate" | ||
| ] |
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.
Can we add "mean", "average", "avg", and "expected" in its keywords?
| var sigma = uniform( 10, 0.1, 20.0, opts ); | ||
| var mu = new Float64Array( sigma.length ); | ||
|
|
||
| var i; | ||
| for ( i = 0; i < sigma.length; i++ ) { | ||
| mu[ i ] = mean( sigma[ i ] ); | ||
| } | ||
|
|
||
| logEach( 'σ: %0.4f, E(X;σ): %0.4f', sigma, mu ); |
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.
Today, we prefer to use logEachMap instead of a manual loop with logEach. This simplifies the code:
| var sigma = uniform( 10, 0.1, 20.0, opts ); | |
| var mu = new Float64Array( sigma.length ); | |
| var i; | |
| for ( i = 0; i < sigma.length; i++ ) { | |
| mu[ i ] = mean( sigma[ i ] ); | |
| } | |
| logEach( 'σ: %0.4f, E(X;σ): %0.4f', sigma, mu ); | |
| var sigma = uniform( 10, 0.1, 20.0, opts ); | |
| logEachMap( 'σ: %0.4f, E(X;σ): %0.4f', sigma, mean ); |
You'd also need to change the import from logEach to logEachMap and remove the Float64Array import.
| if ( v === expected[i] || delta <= tol ) { | ||
| t.strictEqual( v, expected[i], 'sigma: '+sigma[i] ); | ||
| } else { | ||
| t.ok( delta <= tol, 'within tolerance. σ: '+v ); |
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.
The error message here has a bug - it says σ: '+v but v is the computed mean, not the sigma value. Also, the message is missing the other debugging info like expected value and delta. Compare with normal/mean/test/test.js for the full format:
| t.ok( delta <= tol, 'within tolerance. σ: '+v ); | |
| t.ok( delta <= tol, 'within tolerance. σ: '+sigma[i]+'. y: '+v+'. E: '+expected[i]+'. Δ: '+delta+'. tol: '+tol+'.' ); |
Progresses: #9416
Description
This pull request:
meancalculation forhalf-normaldistribution function.Related Issues
This pull request has the following related issues:
@stdlib/stats/base/dists/halfnormalpackage #9416Questions
No.
Other
No.
Checklist
AI Assistance
If you answered "yes" above, how did you use AI assistance?
Disclosure
Yes I took help of AI for creating test cases under the
fixtures/juliadirectory.{{TODO: add disclosure if applicable}}
@stdlib-js/reviewers