Add scipy.linalg.lu() decomposition support#2787
Add scipy.linalg.lu() decomposition support#2787abagusetty wants to merge 4 commits intoIntelPython:masterfrom
scipy.linalg.lu() decomposition support#2787Conversation
abagusetty
requested review from
antonwolfy,
ndgrigorian and
vlad-perevezentsev
as code owners
|
Can one of the admins verify this patch? |
antonwolfy
left a comment
antonwolfy
left a comment
There was a problem hiding this comment.
@abagusetty, thank you for the contribution. I posted few comments below
| from ._decomp_lu import lu, lu_factor, lu_solve | ||
|
|
||
| __all__ = [ | ||
| "lu", |
There was a problem hiding this comment.
Could you please state the change in the CHANGELOG.md?
| low = dpnp.ones( | ||
| a.shape, | ||
| dtype=res_type, | ||
| usm_type=a_usm_type, | ||
| sycl_queue=a_sycl_queue, | ||
| ) |
There was a problem hiding this comment.
it seems can be done through ones_like:
| low = dpnp.ones( | |
| a.shape, | |
| dtype=res_type, | |
| usm_type=a_usm_type, | |
| sycl_queue=a_sycl_queue, | |
| ) | |
| low = dpnp.ones_like(a, dtype=res_type) |
| usm_type=a_usm_type, | ||
| sycl_queue=a_sycl_queue, | ||
| ) | ||
| up = dpnp.array(a, dtype=res_type) |
There was a problem hiding this comment.
looks like scipy might return input array a if dtype matches expected and overwrite_a is True:
| up = dpnp.array(a, dtype=res_type) | |
| up = dpnp.astype(a, dtype=res_type, copy=not overwrite_a) |
| inv_perm = dpnp.zeros( | ||
| (*batch_shape, 1), | ||
| dtype=dpnp.int64, | ||
| usm_type=a_usm_type, | ||
| sycl_queue=a_sycl_queue, | ||
| ) |
There was a problem hiding this comment.
we can use zeros_like here also (that will simplify the code):
| inv_perm = dpnp.zeros( | |
| (*batch_shape, 1), | |
| dtype=dpnp.int64, | |
| usm_type=a_usm_type, | |
| sycl_queue=a_sycl_queue, | |
| ) | |
| inv_perm = dpnp.zeros_like( | |
| a, | |
| shape=(*batch_shape, 1), | |
| dtype=dpnp.int64, | |
| ) |
| ) | ||
|
|
||
| # ---- Fast path: empty arrays ---- | ||
| elif a.size == 0: |
There was a problem hiding this comment.
we can use empty_like for below calls
|
|
||
| # The permutation matrix P uses a real dtype (SciPy convention): | ||
| # P only contains 0s and 1s, so complex storage would be wasteful. | ||
| real_type = _get_real_dtype(res_type) |
There was a problem hiding this comment.
Can we reuse _real_type here instead?
| real_type = _get_real_dtype(res_type) | |
| real_type = _real_type(res_type) |
| usm_type=a_usm_type, | ||
| sycl_queue=a_sycl_queue, | ||
| ) | ||
|
|
There was a problem hiding this comment.
Would it be better (and faster) just to return here and in scalar path above to avoid possible below calls of _apply_permutation_to_rows?
| a[1, 0, 0] = dpnp.nan | ||
| assert_raises(ValueError, dpnp.scipy.linalg.lu, a, check_finite=True) | ||
|
|
||
|
|
There was a problem hiding this comment.
Please also enable muted corresponded tests in dpnp/tests/third_party/cupyx/scipy_tests/linalg_tests/test_decomp_lu.py
| return mat[perm_indices] | ||
|
|
||
| # Batched case: ensure *mat* has the same batch dimensions as | ||
| # *perm_indices*. This is needed, for example, when permuting |
There was a problem hiding this comment.
| # *perm_indices*. This is needed, for example, when permuting | |
| # *perm_indices*. This is needed, for example, when permuting |
3 participants
This PR adds
dpnp.scipy.linalg.lu()with support for all three output modes: default(P, L, U),permute_l=True (PL, U), andp_indices=True(p, L, U), including batched inputs.Fixes: #2786