-
-
Notifications
You must be signed in to change notification settings - Fork 596
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
feat: add C ndarray
implementation for blas/base/zscal
#4864
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: 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: passed - task: lint_javascript_benchmarks status: na - task: lint_python status: na - task: lint_r status: na - task: lint_c_src status: missing_dependencies - task: lint_c_examples status: missing_dependencies - task: lint_c_benchmarks status: missing_dependencies - 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: na - task: run_julia_benchmarks status: na - task: run_python_benchmarks status: na - task: run_r_benchmarks status: na - task: run_javascript_tests status: na ---
Coverage Report
The above coverage report was generated for the changes in this PR. |
var ix; | ||
var i; | ||
|
||
if ( N <= 0 ) { | ||
return zx; | ||
} | ||
// Reinterpret the input array as a real-valued array of interleaved real and imaginary components: | ||
view = reinterpret( zx, 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.
@aman-095 Please revert this refactoring. I just recently updated this to avoid complex number instantiation. Don't simply copy-paste from cscal
.
@@ -32,7 +32,7 @@ var ndarray = require( './ndarray.js' ); | |||
* @param {PositiveInteger} N - number of indexed elements | |||
* @param {Complex128} za - constant | |||
* @param {Complex128Array} zx - input array | |||
* @param {integer} strideZX - `zx` stride length | |||
* @param {integer} strideX - `zx` stride length |
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.
FTR, I recently changed these: c97212e#diff-1135e703968b45bc6a897ad9dc728fb0d90a6b633a520e9c455d7bc6c13a78caR36
Namely, because we are not consistent. Personally, instead of za
and zx
, I'd prefer if we just used alpha
and x
for both real and complex. The use of za
, ca
, etc, stems from Fortran in order to signal the dtype. I think we just create more work for ourselves when going from real to complex by following suit and renaming variables from x
to cx
or zx
. The function name and the C type signatures are enough to convey what is expected. I don't think we need to encode the type info directly into the parameter names themselves.
t.end(); | ||
}); | ||
|
||
tape( 'the function supports a negative `zx` stride', function test( t ) { |
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.
@aman-095 Why is this added given L250 which tests the same thing?
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.
Left an initial round of comments.
Resolves #2039.
Description
This pull request adds C
ndarray
implementation forzscal
.Related Issues
This pull request:
Questions
No.
Other
No.
Checklist
@stdlib-js/reviewers