Update QR tests to avoid element-wise comparisons#2785
Update QR tests to avoid element-wise comparisons#2785vlad-perevezentsev wants to merge 5 commits intomasterfrom
Conversation
|
Array API standard conformance tests for dpnp=0.20.0dev3=py313h509198e_11 ran successfully. |
vlad-perevezentsev
changed the title
|
View rendered docs @ https://intelpython.github.io/dpnp/pull/2785/index.html |
vlad-perevezentsev
requested review from
antonwolfy and
ndgrigorian
as code owners
| ) | ||
| ) | ||
| class TestQRDecomposition(unittest.TestCase): | ||
| def _gram(self, x, xp): |
There was a problem hiding this comment.
Update CHANGELOG.md
|
|
||
| if mode in ("complete", "reduced"): | ||
| q_gpu, r_gpu = result_gpu | ||
| testing.assert_allclose(q_gpu @ r_gpu, a_gpu, atol=1e-4) |
There was a problem hiding this comment.
Why do we need to cast the array to the host memory, when both were allocating on the device?
| elif mode == "raw": | ||
| h_gpu, tau_gpu = result_gpu | ||
| h_cpu, tau_cpu = result_cpu | ||
| m, n = a_gpu.shape[-2], a_gpu.shape[-1] |
There was a problem hiding this comment.
| m, n = a_gpu.shape[-2], a_gpu.shape[-1] | |
| m, n = a_gpu.shape[-2:] |
| testing.assert_allclose( | ||
| self._gram(r_gpu, cupy), exp_gpu, atol=1e-4, rtol=1e-4 | ||
| ) | ||
| testing.assert_allclose( |
There was a problem hiding this comment.
We no need to test NumPy
| if tau_cpu.dtype == numpy.float64: | ||
| tau_cpu = tau_cpu.astype("float32") | ||
| elif tau_cpu.dtype == numpy.complex128: | ||
| tau_cpu = tau_cpu.astype("complex64") | ||
| assert tau_gpu.dtype == tau_cpu.dtype |
There was a problem hiding this comment.
We no need to cast the array:
| if tau_cpu.dtype == numpy.float64: | |
| tau_cpu = tau_cpu.astype("float32") | |
| elif tau_cpu.dtype == numpy.complex128: | |
| tau_cpu = tau_cpu.astype("complex64") | |
| assert tau_gpu.dtype == tau_cpu.dtype | |
| assert tau_gpu.dtype.kind == tau_cpu.dtype.kind | |
| else: | |
| assert tau_gpu.dtype == tau_cpu.dtype |
| testing.assert_allclose( | ||
| self._gram(r_gpu, cupy), exp_gpu, atol=1e-4, rtol=1e-4 | ||
| ) | ||
| testing.assert_allclose( |
There was a problem hiding this comment.
No need to test NumPy behavior along
|
|
||
| class TestQr: | ||
| @pytest.mark.parametrize("dtype", get_all_dtypes(no_bool=True)) | ||
| def gram(self, X, xp): |
There was a problem hiding this comment.
The functions are exactly the same, Is there a way to move them to some helper file to reduce code duplications?
| self.check_qr(a, ia, mode) | ||
|
|
||
| @pytest.mark.parametrize("dtype", get_all_dtypes(no_bool=True)) | ||
| @pytest.mark.parametrize("dtype", get_float_complex_dtypes()) |
There was a problem hiding this comment.
Why do you propose to reduce the scope of verifying dtypes?
3 participants
This PR proposes updating QR tests to avoid direct element-wise comparisons which became unstable with oneMKL 2026.0 due to sign and phase differences in otherwise valid QR results
Since QR factorization is not unique, different MKL and NumPy versions may return results that differ by sign or complex phase while still representing a correct decomposition
To make the tests more stable this PR proposes using invariant-based validation for
mode="raw"andmode="r"based on the unitarity of the Q factor (Q^H Q = I) and the resulting QR identityR^H @ R = A^H @ A