-
-
Notifications
You must be signed in to change notification settings - Fork 870
feat: add ndarray/base/nullary-strided1d
#7772
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
--- 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: passed - task: lint_package_json status: passed - task: lint_repl_help status: passed - task: lint_javascript_src status: passed - task: lint_javascript_cli status: na - task: lint_javascript_examples status: passed - task: lint_javascript_tests status: passed - task: lint_javascript_benchmarks status: na - 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. |
lib/node_modules/@stdlib/ndarray/base/nullary-strided1d/lib/1d.js
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/ndarray/base/nullary-strided1d/lib/2d.js
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/ndarray/base/nullary-strided1d/lib/2d_blocked.js
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/ndarray/base/nullary-strided1d/lib/3d.js
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/ndarray/base/nullary-strided1d/lib/3d_blocked.js
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/ndarray/base/nullary-strided1d/lib/factory.js
Outdated
Show resolved
Hide resolved
Signed-off-by: Muhammad Haris <[email protected]>
--- 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: passed - task: lint_package_json status: na - task: lint_repl_help status: na - task: lint_javascript_src status: passed - task: lint_javascript_cli status: na - task: lint_javascript_examples status: passed - task: lint_javascript_tests status: na - task: lint_javascript_benchmarks status: na - 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 ---
…tdlib into nullary-strided1d
--- 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: passed - task: lint_package_json status: na - task: lint_repl_help status: na - task: lint_javascript_src status: passed - task: lint_javascript_cli status: na - task: lint_javascript_examples status: na - task: lint_javascript_tests status: na - task: lint_javascript_benchmarks status: na - 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 ---
lib/node_modules/@stdlib/ndarray/base/nullary-strided1d/README.md
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/ndarray/base/nullary-strided1d/README.md
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/ndarray/base/nullary-strided1d/README.md
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/ndarray/base/nullary-strided1d/README.md
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/ndarray/base/nullary-strided1d/README.md
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/ndarray/base/nullary-strided1d/lib/nd.js
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/ndarray/base/nullary-strided1d/lib/nd.js
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/ndarray/base/nullary-strided1d/package.json
Outdated
Show resolved
Hide resolved
@kgryte let me know if should go ahead and apply the suggestions above. The suggested change for all is to change |
@headlessNode How about just saying |
The input/output qualifier is less applicable here, as the input array is really treated as both. |
@kgryte Makes sense. |
--- 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: passed - task: lint_package_json status: passed - task: lint_repl_help status: passed - task: lint_javascript_src status: passed - task: lint_javascript_cli status: na - task: lint_javascript_examples status: na - task: lint_javascript_tests status: na - task: lint_javascript_benchmarks status: na - 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 ---
for ( i1 = 0; i1 < S1; i1++ ) { | ||
for ( i0 = 0; i0 < S0; i0++ ) { | ||
setViewOffsets( views, iv ); | ||
v[ 0 ] = strategyX( views[ 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.
The main issue with the current implementation is you need to more closely follow unary-strided1d
in terms of having both an input and output strategy. It is not enough to simply have an input strategy.
Why? Because the input strategy could be to copy data to a workspace. If the strided array function mutates that data (e.g., sorting), then we need to copy that data back from the workspace to the original input array. Right now, your implementation does not handle that. You only handle the input strategy, but not the output.
In short, "nullary" effectively assumes that a provided ndarray is both an input and an output array.
Accordingly, I suggest copying, more or less, the strategy.js
file from unary-strided1d
and then updating all your loop kernels accordingly.
@headlessNode Also, as an aside, I see that you went through each file and added various change requests (over 100 of them). That follow-up is certainly appreciated; however, I might suggest for repeated changes like that, just doing that review in VSCode and pushing up the changes all in one go, potentially by using find-and-replace. In this case, GitHub was not particularly adept at determining that you had resolved all those change requests, requiring me (or someone else) to mark them as "resolved". |
type: pre_commit_static_analysis_report
description: Results of running static analysis checks when committing changes. report:
Progresses #2656.
Description
This pull request:
ndarray/base/nullary-strided1d
Related Issues
This pull request:
Questions
No.
Other
No.
Checklist
@stdlib-js/reviewers