Skip to content
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

add/fix things related to tensors and BLAS #228

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jeffhammond
Copy link

@jeffhammond jeffhammond commented May 10, 2022

  1. infintely is spelled wrong but also wrong in meaning. there are not infinitely more possibilities for tensors, merely combinatorially many more.
  2. tensors over BLAS is not optimal although it is common.
  3. added citations related to tensors and BLAS.
  4. GotoBLAS is obsolete. added OpenBLAS instead.
  5. added BLIS and citation.

infintely is spelled wrong but also wrong in meaning.  there are not infinitely more possibilities for tensors, merely combinatorially many more.
1. tensors over BLAS is not optimal although it is common.
2. added citations related to tensors and BLAS.
3. GotoBLAS is obsolete.  added OpenBLAS instead.
4. added BLIS and citation.
@jeffhammond jeffhammond changed the title fix infintely add/fix things related to tensors and BLAS May 10, 2022
BLAS is an API.  Netlib is an implementation.
the vendors are implementing the BLAS API.
they may or may not use code from Netlib
(in some cases we know, in others not).

also note that AMD forked BLIS, which is now
cited elsewhere.  AMD did not create BLIS.
Copy link
Contributor

@mhoemmen mhoemmen left a comment

Choose a reason for hiding this comment

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

Thanks @jeffhammond for the fixes! We should clarify that only P1673.bs needs to be edited. I'm not sure why we haven't deleted the old file blas_interface.md yet. If you have the time, it would be excellent to delete it in this PR.

@@ -834,12 +835,16 @@ to some multi-indices in the Cartesian product of extents.
### Tensors

We exclude tensors from this proposal, for the following reasons.
First, tensor libraries naturally build on optimized dense linear
First, tensor libraries often build on optimized dense linear
algebra libraries like the BLAS, so a linear algebra library is a good
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
algebra libraries like the BLAS, so a linear algebra library is a good
algebra libraries like the BLAS. Thus, a linear algebra library is a good

@@ -1597,7 +1602,7 @@ pioneering efforts and history lessons.
SLATE Working Notes, Innovative Computing Laboratory, University of
Tennessee Knoxville, Feb. 2018.

* K. Goto and R. A. van de Geijn, "Anatomy of high-performance matrix
* K. Goto and R. A. van de Geijn, ["Anatomy of high-performance matrix
Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch!

@mhoemmen
Copy link
Contributor

I added PR #231, which deletes the old blas_interface.md file. I can also check P1674 to see whether we could apply the changes there.

@mhoemmen
Copy link
Contributor

( @crtrott @nliber @dalg24 )

mhoemmen added a commit to mhoemmen/cpp-proposals-pub that referenced this pull request May 11, 2022
PR ORNL#228 by Jeff Hammond suggests changes to P1673.
Some of those can be applied to P1674 as well.
This PR does that.
@mhoemmen
Copy link
Contributor

@jeffhammond FYI, we applied these changes to P1674, and deleted the old blas_interface.md file. We can leave this PR open as a reminder to make the same changes to P1673. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants