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 specification for computing the qr factorization #126

Merged
merged 6 commits into from
May 12, 2021
Merged

Add specification for computing the qr factorization #126

merged 6 commits into from
May 12, 2021

Conversation

kgryte
Copy link
Contributor

@kgryte kgryte commented Feb 15, 2021

This PR

  • specifies the interface for computing the qr factorization.
  • is derived from comparing signatures across array libraries.

Notes

  • Following Torch, MXNet, and TF, this proposal allows for providing a stack of square matrices. NumPy, Dask, and CuPy do not currently support providing stacks.

  • NumPy supports multiple factorization "modes": raw, reduced, complete, and r modes (along with several legacy modes). Dask does not support any modes. TF only supports reduced and complete modes. MXNet only supports reduced mode. Torch linalg.qr (latest master) does not support raw mode.

    This proposal only includes support for reduced and complete modes.

  • NumPy et al provide support for factorization modes via a mode keyword. TF supports modes via a full_matrices keyword, similar to SVD. This proposal uses a mode keyword for possible future support of additional modes.

  • While Torch linalg.qr supports r mode, it still returns an empty array for q (due to easier JIT). As this proposal does not include r mode support, a namedtuple containing q and r is always returned.

  • NumPy, CuPy (?), and JAX support returning h and tau arrays containing the Householder reflectors and associated scaling factors, respectively. TF and Torch do not. This proposal does not support returning h and tau.

Copy link
Member

@rgommers rgommers left a comment

Choose a reason for hiding this comment

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

Thanks @kgryte. I agree with all the choices made.

@rgommers rgommers added the API extension Adds new functions or objects to the API. label Mar 20, 2021
@kgryte
Copy link
Contributor Author

kgryte commented May 12, 2021

Thanks, @leofang and @rgommers, for the reviews! This PR is ready for merge...

@kgryte kgryte merged commit a33a710 into main May 12, 2021
@kgryte kgryte deleted the qr branch May 12, 2021 04:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API extension Adds new functions or objects to the API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants