-
-
Notifications
You must be signed in to change notification settings - Fork 896
feat: add lapack/base/dge-trans
#2734
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
Conversation
@Pranavchiku Is there a reason to name the package as transpose rather than dge-trans? We'll need separate packages for single-precision and complex dtypes.
I see, I initially used snake case and it threw me lint error. Also, I am unable to understand sense of dge-*, can we not have dtrans?
Is it double precision general matrix transpose? If yes, then it makes sense.
2a166a3 to
e69805a
Compare
lapack/base/transpose (削除ここまで)lapack/base/dge-trans (追記ここまで)
Is it double precision general matrix transpose? If yes, then it makes sense.
Yes, I believe so. Not sure why they didn't do dgetrans, but 🤷♂️ .
lapack/base/dge-trans (削除ここまで)lapack/base/dgetrans (追記ここまで)
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 some initial comments.
3868710 to
fa8d1a2
Compare
lapack/base/dgetrans (削除ここまで)lapack/base/dge-trans (追記ここまで)
.../Pranavchiku/2734
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.
LGTM. Thanks for working on this, @Pranavchiku.
Note that the tests were rather spartan. I would argue it is better to create tests which build on one another, rather than creating a single test which attempts to do all the things. By isolating one behavioral aspect at a time, we can better ensure that an implementation behaves as expected. For combining all the things into one, it can be harder to disentangle which aspect is causing a test to fail.
In principle, we could implement matrix transposition using loop tiling. Loop tiling would only benefit when layout access patterns for A and out differ. E.g., if both A and out are the same (e.g., row-major), because we are taking the transpose, the access pattern for A will be column-major. In this instance, loop tiling can be beneficial. Otherwise, a naive nested loop is cache optimal. Regardless, I'll leave implementing loop tiling to a follow-up PR.
PR-URL: stdlib-js#2734 Ref: stdlib-js#2464 Co-authored-by: Athan Reines <kgryte@gmail.com> Reviewed-by: Athan Reines <kgryte@gmail.com>
Uh oh!
There was an error while loading. Please reload this page.
Towards #2464.
Description
This pull request exposes utility function
dge_transaslapack/base/dge-trans.Related Issues
NA
Questions
No.
Other
No.
Checklist
@stdlib-js/reviewers