array-api-compat icon indicating copy to clipboard operation
array-api-compat copied to clipboard

Support for xp.linalg.lu (not yet part of the Array API spec)

Open ogrisel opened this issue 2 years ago • 5 comments

Context adding LU factorization to the Array API spec:

  • https://github.com/data-apis/array-api/issues/627

Since numpy.linalg.lu does not exist yet (but scipy.linalg.lu does), @rgommers suggested working out an API candidate as part of array-api-compat first by reviewing the API choices and options implemented in various libraries targeting Array API support.

A short term implementation for numpy could probably delegate the work to scipy.linalg.lu in a first iteration.

Based on this work, numpy.linalg could subsequently be extended to directly implement the correct API to be submitted for a future revision of the spec.

Motivation: this is needed to add Array API support to the randomized linear algebra solver for the Principal Component Analysis estimator in scikit-learn:

  • https://github.com/scikit-learn/scikit-learn/pull/26315

Non exhaustive list of available implementations:

Related methods in scipy:

ogrisel avatar May 17 '23 12:05 ogrisel

Thanks @ogrisel! It looks like you only have a single usage in scikit-learn (here), with lu(x, permute_l=True), so if that is covered you should be good for now, right?

I think for the API design decisions, it's best to put them on https://github.com/data-apis/array-api/issues/627. And then let's implement it here. I'll try to post some thoughts there within the next few hours.

rgommers avatar May 17 '23 12:05 rgommers

It looks like you only have a single usage in scikit-learn (here), with lu(x, permute_l=True), so if that is covered you should be good for now, right?

Yes, this is also what I found out via git grep.

ogrisel avatar May 17 '23 13:05 ogrisel

If numpy support relied scipy.linalg.lu(), could this be implemented by conditionally adding lu() to numpy-compat's linalg namespace only when scipy is installed, and not making scipy a requirement?

ntessore avatar May 17 '23 14:05 ntessore

That sounds like the right thing to do to me, we don't want this package to have a direct dependency on scipy.

rgommers avatar May 17 '23 15:05 rgommers

Based on the scope of array-api-compat, lu should be added first to a draft version of the standard. Then we can add it here. It's better to discuss design specifics of it in the array-api repo. For actual implementations, it would be better to discuss that with specific libraries.

Regarding the scipy question, if lu were added, I wouldn't see an issue with making only that function (in the numpy wrapper) depend on scipy.

asmeurer avatar Mar 27 '24 22:03 asmeurer