Skip to content

feat: add blas/base/ssymv #2305

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

Merged
merged 37 commits into from
Jun 20, 2024
Merged

feat: add blas/base/ssymv #2305

merged 37 commits into from
Jun 20, 2024

Conversation

aman-095
Copy link
Member

@aman-095 aman-095 commented Jun 5, 2024

Description

What is the purpose of this pull request?

This RFC proposes to add a routine to perform one of the matrix-vector operations y = alpha*A*x + beta*y where alpha and beta are scalars, x and y are n element vectors and A is an n by n symmetric matrix as defined in BLAS Level 2 routines. Specifically adding @stdlib/blas/base/ssymv is proposed.

Related Issues

Does this pull request have any related issues?

None.

Questions

Any questions for reviewers of this pull request?

No.

Other

Any other information relevant to this pull request? This may include screenshots, references, and/or implementation notes.

No.

Checklist

Please ensure the following tasks are completed before submitting this pull request.


@stdlib-js/reviewers

@stdlib-bot stdlib-bot added the BLAS Issue or pull request related to Basic Linear Algebra Subprograms (BLAS). label Jun 5, 2024
@kgryte
Copy link
Member

kgryte commented Jun 8, 2024

@aman-095 I've done a partial review and refactor: benchmarks, docs, examples, and ssymv.js. You should pay particular attention to ssymv.js. I've leave it to you to apply to ndarray.js and to update the rest of the documentation and tests.

@kgryte kgryte added Needs Changes Pull request which needs changes before being merged. Feature Issue or pull request for adding a new feature. labels Jun 8, 2024
@aman-095
Copy link
Member Author

aman-095 commented Jun 8, 2024

Sure, Athan will review those and apply changes elsewhere. 

@kgryte
Copy link
Member

kgryte commented Jun 20, 2024

@aman-095 One thing which is missing from the tests are column-major variants. All test fixtures atm are row-major.

@kgryte
Copy link
Member

kgryte commented Jun 20, 2024

@aman-095 We also need to add assertion tests in order to get 100% code coverage.

Copy link
Member

@kgryte kgryte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks, @aman-095.

For the other related PRs, you'll want to compute the diff between your last commit and the current state. That will help inform you as to what changes need to be propagated to ssbmv and others.

Note that I went ahead and added tests for edge cases, assertions, and column-major. Those will also need to be added to other PRs.

@kgryte kgryte merged commit 62744b5 into stdlib-js:develop Jun 20, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BLAS Issue or pull request related to Basic Linear Algebra Subprograms (BLAS). Feature Issue or pull request for adding a new feature. Needs Changes Pull request which needs changes before being merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants