bench: refactor random number generation in stats/base/dists/laplace#5270
bench: refactor random number generation in stats/base/dists/laplace#5270Planeshifter merged 7 commits intostdlib-js:developfrom
stats/base/dists/laplace#5270Conversation
…ase/dists/laplace
Coverage Report
The above coverage report was generated for the changes in this PR. |
|
@anandkaranubc @kgryte please review this PRs |
|
@anandkaranubc @Planeshifter ready for review |
|
@anandkaranubc please review this PRs |
On it! |
thanks |
lib/node_modules/@stdlib/stats/base/dists/laplace/cdf/benchmark/benchmark.js
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/stats/base/dists/laplace/cdf/benchmark/benchmark.js
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/stats/base/dists/laplace/ctor/benchmark/benchmark.js
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/stats/base/dists/laplace/pdf/benchmark/benchmark.js
Outdated
Show resolved
Hide resolved
|
@gkbishnoi07, Can you also make the same changes that were made in #5296? Specifically, the The title should be updated to (for consistency): bench: refactor random number generation in |
Sure, I'll update you soon. |
stats/base/dists/laplacestats/base/dists/laplace
|
@anandkaranubc please review this PRs |
@gkbishnoi07 Can you point out the files that need this change? Note that this change is only needed where the random number generation occurs outside the benchmarking loops. Why? Because in cases like these, specifically in the |
@anandkaranubc Got it! I just noticed that some files still have random generation inside the benchmarking loop instead of using a uniform distribution. First, I'm working on updating those files, and along the way, I'll implement insan(dist.*). |
Yes, and this change is being tracked in #4993. Ideally, this change should already be reflected in the respective PRs. If you have other PRs related to this tracking issue, please update them as well. Thanks! |
Ok sure! |
|
@anandkaranubc, are you reviewing this PR now? |
|
@anandkaranubc i applied all changes please look out this PR |
|
@anandkaranubc please review this PRs |
On it! |
lib/node_modules/@stdlib/stats/base/dists/laplace/ctor/benchmark/benchmark.js
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/stats/base/dists/laplace/ctor/benchmark/benchmark.js
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/stats/base/dists/laplace/ctor/benchmark/benchmark.js
Show resolved
Hide resolved
lib/node_modules/@stdlib/stats/base/dists/laplace/median/benchmark/benchmark.js
Outdated
Show resolved
Hide resolved
|
@gkbishnoi07 The PR still needs some changes. |
|
@anandkaranubc changes applied please review this PRs |
lib/node_modules/@stdlib/stats/base/dists/laplace/ctor/benchmark/benchmark.js
Outdated
Show resolved
Hide resolved
|
It's also good practice to keep your commit messages consistent with code conventions. Here is the guide |
anandkaranubc
left a comment
There was a problem hiding this comment.
Please also confirm if the same change is consistent throughout the PR.
lib/node_modules/@stdlib/stats/base/dists/laplace/quantile/benchmark/benchmark.js
Show resolved
Hide resolved
|
@anandkaranubc please review |
anandkaranubc
left a comment
There was a problem hiding this comment.
Everything looks good now! Thanks for working on this.
|
@anandkaranubc stdlib bot again give Needs Review label? |
Yes, the PR needs to go through the maintainers before it can be merged. |
Okay,thanks |
Planeshifter
left a comment
There was a problem hiding this comment.
Thanks @gkbishnoi07 for your PR and @anandkaranubc for review. Let's get this one in. 🚀
PR Commit MessagePlease review the above commit message and make any necessary adjustments. |
Resolves #4976
Description
What is the purpose of this pull request?
This pull request:
stats/base/dists/laplace.randu()withuniform()for cleaner and more consistent code.Related Issues
Does this pull request have any related issues?
This pull request:
stats/base/dists/laplace#4976Questions
Any questions for reviewers of this pull request?
No.
Other
Any other information relevant to this pull request?
No.
Checklist
Please ensure the following tasks are completed before submitting this pull request.