Skip to content

Remove linalg.lstsq #240

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 1 commit into from
Aug 23, 2021
Merged

Remove linalg.lstsq #240

merged 1 commit into from
Aug 23, 2021

Conversation

kgryte
Copy link
Contributor

@kgryte kgryte commented Aug 16, 2021

This PR

  • removes linalg.lstsq (per consensus vote held during the consortium meeting held 2021-07-29). This PR supersedes gh-235, which attempted to replace lstsq with a simpler API.
  • closes gh-227 and gh-235.

Background

Initial discussion on linalg.lstsq API design can be found in gh-227. There, it was held that the current API design posed undue burden on specification implementers due to API complexity (returning solutions, ranks, residuals, and singular values) and violated various linear algebra API design principles (particularly orthogonality).

Based on ensuing discussion in consortium meetings, consensus was found in simply removing the linalg.lstsq API altogether. This API finds low usage among downstream libraries (see data) and has several variants due to specialized use cases (see SciPy).

Should this or similar functionality be found to be desirable in the future, specification can happen in a future revision of the standard.

@kgryte
Copy link
Contributor Author

kgryte commented Aug 23, 2021

As this change was discussed and agreed upon in the consortium meeting held 2021-07-29, will merge. As mentioned in the OP, re-adding to the specification at a future date can be considered as part of a future revision of this standard.

@kgryte kgryte merged commit 3002279 into main Aug 23, 2021
@kgryte kgryte deleted the remove-lstsq branch August 23, 2021 18:12
@rgommers rgommers mentioned this pull request Mar 14, 2022
@rgommers rgommers added the topic: Linear Algebra Linear algebra. label May 11, 2023
@rgommers
Copy link
Member

I'll note that this came up in #403 (comment), quoting:

I realize that lstsq was removed as per a previous discussion (and it's understandable given that the API is a bit crufty), but then our code base has 26 unique call sites of that alone, since we're dealing mostly with engineering and stats. One might reasonably assume that backends that already have optimized implementations of that (all 6 do, and torch/tf support batched matrices) will provide it anyway in their array API namespace. However, we do worry that, given that it has been deliberately removed, we can't be sure that some of the existing backends won't be encouraged to minimize their maintenance surface and drop functions like that from their formerly numpy-compatible (soon array API compatible) namespace, forcing users like us to deal with their raw EagerTensor, xla_extension.DeviceArray, or whatever it may be called, and go find it in whichever ancestral tensorflow namespace the functionality may have been buried in before. We're wondering if a tradeoff could be made where e.g., some of the rarely-used outputs could be marked as as "reserved" placeholder and are allowed to hold unspecified values (e.g., None) until perhaps at some future date the API specifies them. There's also the option to go the same route as with svd, where some arguments were removed in the interest of simplicity.

It also came up in SciPy, where it was hit in one of the first APIs on which a conversion was tried (signal.welch): scipy/scipy#18286 (comment)

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

Successfully merging this pull request may close these issues.

Return just the solution in linalg.lstsq
2 participants