feat: add ml/strided/dkmeans-compute-inertia#13004
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_pkg_readmes
status: passed
- task: lint_markdown_docs
status: na
- task: lint_markdown
status: na
- 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: passed
- task: lint_python
status: na
- task: lint_r
status: na
- task: lint_c_src
status: passed
- task: lint_c_examples
status: passed
- task: lint_c_benchmarks
status: passed
- task: lint_c_tests_fixtures
status: na
- task: lint_shell
status: na
- task: lint_typescript_declarations
status: passed
- task: lint_typescript_tests
status: passed
- task: lint_license_headers
status: passed
---
Coverage Report
The above coverage report was generated for the changes in this PR. |
| * Compute inertia between double-precision floating-point centroids and data points. | ||
| * | ||
| * @private | ||
| * @param {PositiveInteger} M - number of samples. |
There was a problem hiding this comment.
| * @param {PositiveInteger} M - number of samples. | |
| * @param {PositiveInteger} M - number of samples |
Parameter descriptions should not end in periods. This is project convention. You should update all similar descriptions accordingly. This likely applies to your other PRs, as well.
| * @param {Float64Array} C - strided array centroid locations. | ||
| * @param {integer} sc1 - stride length of first dimension of c. | ||
| * @param {integer} sc2 - stride length of second dimension of c. | ||
| * @param {integer} oc - initial index of centroids. |
There was a problem hiding this comment.
| * @param {integer} oc - initial index of centroids. | |
| * @param {integer} oc - starting index of `C` |
Use consistent descriptions.
| * @param {integer} oc - initial index of centroids. | ||
| * @param {Int32Array} labels - labels array containing cluster index of each data point. | ||
| * @param {integer} sl - stride length of labels. | ||
| * @param {integer} ol - initial index of labels. |
There was a problem hiding this comment.
| * @param {integer} ol - initial index of labels. | |
| * @param {integer} ol - starting index of `labels` |
In JS parameter descriptions, we place references to other parameter names in backticks.
| if ( metric === 'sqeuclidean' ) { | ||
| dist = dsquaredEuclidean; | ||
| } else if ( metric === 'correlation' ) { | ||
| dist = dcorrelation; | ||
| } else if ( metric === 'cityblock' ) { | ||
| dist = dcityblock; | ||
| } else { | ||
| dist = dcosine; | ||
| } |
There was a problem hiding this comment.
IIRC, this sort of nested conditional occurs elsewhere. In which case, I am wondering if it would be better to create a package ml/strided/dkmeans-metric2strided (different name?). We kind of have precedent elsewhere (see https://github.com/stdlib-js/stdlib/tree/develop/lib/node_modules/%40stdlib/array/ctors). The API would be
var TABLE = {
'sqeuclidean': dsquaredEuclidean,
'correlation': dcorrelation,
'cityblock': dcityblock,
'cosine': dcosine
};
function metric2strided( metric ) {
return TABLE[ metric ] || null;
}Then, in this implementation, the conditional becomes
// ...
dist = metric2strided( metric );
// ...where we assume that metric is a recognized metric.
There was a problem hiding this comment.
I think this package needs to be renamed: dkmeans-inertia. The use of compute in the name is redundant, IMO. Your API alias then becomes dkmeansInertia.
| CBLAS_INT i; | ||
| double d; | ||
|
|
||
| if ( metric == STDLIB_ML_KMEANS_SQEUCLIDEAN ) { |
There was a problem hiding this comment.
In contrast to JS, I don't think there is a "nice" way of abstracting this logic out into a separate function/macro, as then we'd need to make the func_type public, etc. So, I believe we are left with inlining, as done here.
| } | ||
|
|
||
| /** | ||
| * Compute inertia between double-precision floating-point centroids and data points using alternative indexing semantics. |
There was a problem hiding this comment.
| * Compute inertia between double-precision floating-point centroids and data points using alternative indexing semantics. | |
| * Computes the inertia between double-precision floating-point centroids and data points using alternative indexing semantics. |
| typedef double func_type( const CBLAS_INT N, const double *X, const CBLAS_INT sx, const CBLAS_INT ox, const double *Y, const CBLAS_INT strideY, const CBLAS_INT offsetY ); | ||
|
|
||
| /** | ||
| * Compute inertia between double-precision floating-point centroids and data points. |
There was a problem hiding this comment.
| * Compute inertia between double-precision floating-point centroids and data points. | |
| * Computes the inertia between double-precision floating-point centroids and data points. |
| #include <stdint.h> | ||
|
|
||
| /** | ||
| * Data type to store distance metric function. |
There was a problem hiding this comment.
| * Data type to store distance metric function. | |
| * Type definition for a function which computes a distance metric for an input strided array. |
type: pre_commit_static_analysis_report
description: Results of running static analysis checks when committing changes. report:
Resolves a part of #12875.
Description
This pull request:
ml/strided/dkmeans-compute-inertia.Related Issues
This pull request has the following related issues:
Questions
No.
Other
No.
Checklist
AI Assistance
If you answered "yes" above, how did you use AI assistance?
Disclosure
{{TODO: add disclosure if applicable}}
@stdlib-js/reviewers