-
-
Notifications
You must be signed in to change notification settings - Fork 822
bench: refactor random number generation in stats/base/dists/normal #5112
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?
Conversation
…or stats/base/dists/normal --- type: pre_commit_static_analysis_report description: Results of running static analysis checks when committing changes. report: - task: lint_filenames status: passed - task: lint_editorconfig status: passed - task: lint_markdown status: na - task: lint_package_json status: na - task: lint_repl_help status: na - task: lint_javascript_src status: na - task: lint_javascript_cli status: na - task: lint_javascript_examples status: na - task: lint_javascript_tests status: na - task: lint_javascript_benchmarks status: passed - task: lint_python status: na - task: lint_r status: na - task: lint_c_src status: na - task: lint_c_examples status: na - task: lint_c_benchmarks status: na - task: lint_c_tests_fixtures status: na - task: lint_shell status: na - task: lint_typescript_declarations status: na - task: lint_typescript_tests status: na - task: lint_license_headers status: passed ---
Coverage Report
The above coverage report was generated for the changes in this PR. |
} | ||
|
||
b.toc(); |
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.
} | |
b.toc(); | |
} | |
b.toc(); |
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.
This change is still needed.
} | ||
|
||
b.toc(); |
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.
} | |
b.toc(); | |
} | |
b.toc(); |
Same comment as above, and applies throughout the PR
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.
This still needs to be addressed.
} | ||
b.tic(); |
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.
} | |
b.tic(); | |
} | |
b.tic(); |
mu = ( randu()*100.0 ) - 50.0; | ||
sigma = ( randu()*20.0 ) + EPS; | ||
y = mode( mu, sigma ); | ||
y = mode( mu[ i % 100 ], sigma[ i % 100 ] ); |
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.
y = mode( mu[ i % 100 ], sigma[ i % 100 ] ); | |
y = mode( mu[ i % len ], sigma[ i % len ] ); |
mu = ( randu()*100.0 ) - 50.0; | ||
sigma = ( randu()*20.0 ) + EPS; | ||
y = pdf( x, mu, sigma ); | ||
y = pdf( x[ i % 100 ], mu[ i % 100 ], sigma[ i % 100 ] ); |
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.
Same comment.
|
||
b.tic(); | ||
for ( i = 0; i < b.iterations; i++ ) { | ||
x = ( randu()*6.0 ) - 3.0; | ||
y = mypdf( x ); | ||
y = mypdf( x[ i % 100 ] ); |
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.
y = mypdf( x[ i % 100 ] ); | |
y = mypdf( x[ i % len ] ); |
Applies throughout the PR.
Great work, @pranav-1720! Just a few minor changes needed; otherwise, everything looks good! And also change in title: bench: refactor random number generation in |
--- type: pre_commit_static_analysis_report description: Results of running static analysis checks when committing changes. report: - task: lint_filenames status: passed - task: lint_editorconfig status: passed - task: lint_markdown status: na - task: lint_package_json status: na - task: lint_repl_help status: na - task: lint_javascript_src status: na - task: lint_javascript_cli status: na - task: lint_javascript_examples status: na - task: lint_javascript_tests status: na - task: lint_javascript_benchmarks status: passed - task: lint_python status: na - task: lint_r status: na - task: lint_c_src status: na - task: lint_c_examples status: na - task: lint_c_benchmarks status: na - task: lint_c_tests_fixtures status: na - task: lint_shell status: na - task: lint_typescript_declarations status: na - task: lint_typescript_tests status: na - task: lint_license_headers status: passed ---
--- type: pre_commit_static_analysis_report description: Results of running static analysis checks when committing changes. report: - task: lint_filenames status: passed - task: lint_editorconfig status: passed - task: lint_markdown status: na - task: lint_package_json status: na - task: lint_repl_help status: na - task: lint_javascript_src status: na - task: lint_javascript_cli status: na - task: lint_javascript_examples status: na - task: lint_javascript_tests status: na - task: lint_javascript_benchmarks status: passed - task: lint_python status: na - task: lint_r status: na - task: lint_c_src status: na - task: lint_c_examples status: na - task: lint_c_benchmarks status: na - task: lint_c_tests_fixtures status: na - task: lint_shell status: na - task: lint_typescript_declarations status: na - task: lint_typescript_tests status: na - task: lint_license_headers status: passed ---
@anandkaranubc hey have made the changes you suggested |
} | ||
b.tic(); | ||
for ( i = 0; i < b.iterations; i++ ) { |
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.
} | |
b.tic(); | |
for ( i = 0; i < b.iterations; i++ ) { | |
} | |
b.tic(); | |
for ( i = 0; i < b.iterations; i++ ) { |
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.
This change is still pending
} | ||
|
||
b.toc(); |
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.
This change is still needed.
} | ||
|
||
b.toc(); |
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.
This still needs to be addressed.
--- type: pre_commit_static_analysis_report description: Results of running static analysis checks when committing changes. report: - task: lint_filenames status: passed - task: lint_editorconfig status: passed - task: lint_markdown status: na - task: lint_package_json status: na - task: lint_repl_help status: na - task: lint_javascript_src status: na - task: lint_javascript_cli status: na - task: lint_javascript_examples status: na - task: lint_javascript_tests status: na - task: lint_javascript_benchmarks status: passed - task: lint_python status: na - task: lint_r status: na - task: lint_c_src status: na - task: lint_c_examples status: na - task: lint_c_benchmarks status: na - task: lint_c_tests_fixtures status: na - task: lint_shell status: na - task: lint_typescript_declarations status: na - task: lint_typescript_tests status: na - task: lint_license_headers status: passed --- --- type: pre_push_report description: Results of running various checks prior to pushing changes. report: - task: run_javascript_examples status: na - task: run_c_examples status: na - task: run_cpp_examples status: na - task: run_javascript_readme_examples status: na - task: run_c_benchmarks status: na - task: run_cpp_benchmarks status: na - task: run_fortran_benchmarks status: na - task: run_javascript_benchmarks status: passed - task: run_julia_benchmarks status: na - task: run_python_benchmarks status: na - task: run_r_benchmarks status: na - task: run_javascript_tests status: na ---
@anandkaranubc added the whitespace changes as well. Also is there an automated way to check these whitespace linting isues, the current linting config does not seem to catch them or perhaps i am missing something in my local setup. I get errors for trailing whitespace and some other issues but not the ones you highlighted for this PR |
I don’t think the linter catches all the whitespace linting issues. It does catch some (there might be a reason why it is configured this way). Maybe you can look into improving it, propose an idea in the OH tomorrow, or raise an issue? |
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.
Still some changes to be made. Rest, the PR looks good. Thanks for making those changes!
} | ||
b.tic(); |
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.
} | |
b.tic(); | |
} | |
b.tic(); |
len = 100; | ||
x = new Float64Array( len ); | ||
for ( i = 0; i < len; i++ ) { | ||
x[ i ] = uniform( EPS + 1.0, 100.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.
x[ i ] = uniform( EPS + 1.0, 100.0 ); | |
x[ i ] = uniform( 1.0 + EPS, 100.0 ); |
Not something that makes a difference, but good for consistency.
} | ||
b.tic(); |
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.
} | |
b.tic(); | |
} | |
b.tic(); |
This applies throughout the PR
} | ||
b.tic(); | ||
for ( i = 0; i < b.iterations; i++ ) { |
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.
This change is still pending
@@ -21,7 +21,8 @@ | |||
// MODULES // | |||
|
|||
var bench = require( '@stdlib/bench' ); | |||
var randu = require( '@stdlib/random/base/randu' ); | |||
var Float64Array = require( '@stdlib/array/float64' ); | |||
var uniform = require( '@stdlib/random/base/uniform' ); |
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.
var uniform = require( '@stdlib/random/base/uniform' ); | |
var uniform = require( '@stdlib/random/base/uniform' ); |
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.
fixed, how did you even find that. I dont even know how the space was replaced by tab
@@ -21,7 +21,8 @@ | |||
// MODULES // | |||
|
|||
var bench = require( '@stdlib/bench' ); | |||
var randu = require( '@stdlib/random/base/randu' ); | |||
var Float64Array = require( '@stdlib/array/float64' ); | |||
var uniform = require( '@stdlib/random/base/uniform' ); |
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.
Same comment
--- type: pre_commit_static_analysis_report description: Results of running static analysis checks when committing changes. report: - task: lint_filenames status: passed - task: lint_editorconfig status: passed - task: lint_markdown status: na - task: lint_package_json status: na - task: lint_repl_help status: na - task: lint_javascript_src status: na - task: lint_javascript_cli status: na - task: lint_javascript_examples status: na - task: lint_javascript_tests status: na - task: lint_javascript_benchmarks status: passed - task: lint_python status: na - task: lint_r status: na - task: lint_c_src status: na - task: lint_c_examples status: na - task: lint_c_benchmarks status: na - task: lint_c_tests_fixtures status: na - task: lint_shell status: na - task: lint_typescript_declarations status: na - task: lint_typescript_tests status: na - task: lint_license_headers status: passed ---
--- type: pre_commit_static_analysis_report description: Results of running static analysis checks when committing changes. report: - task: lint_filenames status: passed - task: lint_editorconfig status: passed - task: lint_markdown status: na - task: lint_package_json status: na - task: lint_repl_help status: na - task: lint_javascript_src status: na - task: lint_javascript_cli status: na - task: lint_javascript_examples status: na - task: lint_javascript_tests status: na - task: lint_javascript_benchmarks status: passed - task: lint_python status: na - task: lint_r status: na - task: lint_c_src status: na - task: lint_c_examples status: na - task: lint_c_benchmarks status: na - task: lint_c_tests_fixtures status: na - task: lint_shell status: na - task: lint_typescript_declarations status: na - task: lint_typescript_tests status: na - task: lint_license_headers status: passed ---
@anandkaranubc made the changes, hopefully everything should be in order now. Cheers! |
Resolves #4981.
Description
This pull request:
Related Issues
This pull request:
Questions
No.
Other
No.
Checklist
@stdlib-js/reviewers