search

data-apis / array-api-compat

Compatibility layer for common array libraries to support the Array API
https://data-apis.org/array-api-compat/
MIT License
75 stars 25 forks source link

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

Open ogrisel opened 1 year ago

ogrisel commented 1 year ago

Context adding LU factorization to the Array API spec:

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:

Non exhaustive list of available implementations:

Related methods in scipy:

rgommers commented 1 year ago

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.

ogrisel commented 1 year ago

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.

ntessore commented 1 year ago

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?

rgommers commented 1 year ago

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

asmeurer commented 7 months ago

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.