Skip to content

Named tuple field names of linear algebra functions #295

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

Closed
IvanYashchuk opened this issue Nov 1, 2021 · 4 comments · Fixed by #313
Closed

Named tuple field names of linear algebra functions #295

IvanYashchuk opened this issue Nov 1, 2021 · 4 comments · Fixed by #313
Labels
API change Changes to existing functions or objects in the API. topic: Linear Algebra Linear algebra.
Milestone

Comments

@IvanYashchuk
Copy link

Currently linalg.qr and linalg.svd specify the return tuple field names using lower case letters: q, r, u, s, vh.

Are there any previous discussions on the naming convention? It's common in the linear algebra domain to denote the matrices with upper case single letters. Therefore in PyTorch's linear algebra module (that already uses the namedtuple returns) it was chosen to use qr() -> (Q, R) and svd() -> (U, S, Vh) - upper case letters.

In Julia's LinearAlgebra module also upper case letters are used for accessing the result of QR and SVD decompositions.

@kgryte kgryte added API change Changes to existing functions or objects in the API. topic: Linear Algebra Linear algebra. labels Nov 1, 2021
@kgryte kgryte added this to the v2021 milestone Nov 1, 2021
@kgryte
Copy link
Contributor

kgryte commented Nov 1, 2021

No discussion to which to refer. Using lowercase letters was initially chosen for case consistency throughout the API standard and reduce the need for users to know what case to use when. However, we can obviously revise this and elect to allow tuple fields to be uppercase. We'd just need a list fields which would need to be modified.

@leofang
Copy link
Contributor

leofang commented Nov 1, 2021

I think for consistency with other APIs it's probably better to stay with lowercases. Also save us one key stroke 🙂

@rgommers
Copy link
Member

rgommers commented Nov 1, 2021

I think the upper case names are very reasonable, probably even preferred if we'd do a greenfield design. Now that PyTorch is the only library that implements this already, it certainly makes sense - we didn't check this carefully enough perhaps, and we do not want to force PyTorch to make a backwards-incompatible change here (and namedtuples are limited, adding aliases in there seems tricky).

So I'm +1 for changing this to upper-case.

@kgryte
Copy link
Contributor

kgryte commented Nov 4, 2021

In today's call, we decided to adopt @IvanYashchuk proposal to align with PyTorch in namedtuple field names for linear algebra APIs. I'll submit a PR shortly to update the specification.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API change Changes to existing functions or objects in the API. topic: Linear Algebra Linear algebra.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants