Feature/elementwise functions out keyword by vtavana · Pull Request #1209 · IntelPython/dpctl

vtavana · 2023-05-19T01:55:51Z

Adding out keyword for elementwise functions

Have you provided a meaningful PR description?
Have you added a test, reproducer or referred to an issue with a reproducer?
Have you tested your changes locally for CPU and GPU devices?
Have you made sure that new changes do not introduce compiler warnings?
Have you checked performance impact of proposed changes?
If this PR is a work in progress, are you opening the PR as a draft?

github-actions · 2023-05-19T02:17:09Z

View rendered docs @ https://intelpython.github.io/dpctl/pulls/1209/index.html

github-actions · 2023-05-19T02:50:18Z

Array API standard conformance tests for dpctl=0.14.3dev1=py310h76be34b_98 ran successfully.
Passed: 220
Failed: 780
Skipped: 116

github-actions · 2023-05-19T04:04:19Z

Array API standard conformance tests for dpctl=0.14.3dev1=py310h76be34b_108 ran successfully.
Passed: 226
Failed: 774
Skipped: 116

oleksandr-pavlyk · 2023-05-19T12:54:42Z

In [1]: import dpctl.tensor as dpt, numpy as np

In [2]: m = dpt.ones((10, 10), dtype="f4")

In [3]: v = dpt.ones(10, dtype="i4")

In [4]: r = dpt.empty_like(m)

In [5]: dpt.add(m, v, out=r)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
Input In [5], in <cell line: 1>()
----> 1 dpt.add(m, v, out=r)

File ~/repos/dpctl/dpctl/tensor/_elementwise_common.py:406, in BinaryElementwiseFunc.__call__(self, o1, o2, out, order)
    401     raise TypeError(
    402         f"output array must be of usm_ndarray type, got {type(out)}"
    403     )
    405 if out.shape != o1_shape or out.shape != o2_shape:
--> 406     raise TypeError(
    407         "The shape of input and output arrays are inconsistent."
    408         f"Expected output shape is {o1_shape}, got {out.shape}"
    409     )
    411 if ti._array_overlap(o1, out) or ti._array_overlap(o2, out):
    412     raise TypeError("Input and output arrays have memory overlap")

TypeError: The shape of input and output arrays are inconsistent.Expected output shape is (10, 10), got (10, 10)

github-actions · 2023-05-19T16:35:16Z

Array API standard conformance tests for dpctl=0.14.3dev1=py310h76be34b_108 ran successfully.
Passed: 227
Failed: 773
Skipped: 116

oleksandr-pavlyk · 2023-05-19T19:31:24Z

Changes in this PR lead to a drop in coverage (84.036% in the target branch vs. 83.897% in this branch). Since your changes only modify _elementwise_common.py, please use pytest-cov to identify gaps and add required tests.

vtavana · 2023-05-20T01:03:49Z

Changes in this PR lead to a drop in coverage (84.036% in the target branch vs. 83.897% in this branch). Since your changes only modify _elementwise_common.py, please use pytest-cov to identify gaps and add required tests.

I added new tests and now the coverage for _elementwise_common.py is 91% compared to 89%. All the lines that are added in this PR are tested now.

after adding new tests
dpctl/tensor/_elementwise_common.py 300 26 176 15 91% 83, 151, 161, 185-186, 229, 239, 243, 254, 259, 277, 315, 318, 341, 361, 372-373, 390, 475-477, 487, 517-519, 529

before adding new tests:
dpctl/tensor/_elementwise_common.py 300 30 176 20 89% 83, 95, 119->124, 151, 161, 185-186, 229, 239, 243, 254, 259, 277, 315, 318, 341, 361, 372-373, 390, 459, 475-477, 487, 496, 517-519, 529, 538

github-actions · 2023-05-20T01:38:27Z

Array API standard conformance tests for dpctl=0.14.3dev1=py310h76be34b_110 ran successfully.
Passed: 227
Failed: 773
Skipped: 116

oleksandr-pavlyk · 2023-05-20T15:54:50Z

Tests fail on my WSL where GPU does not support fp64.

Specifically test_add_dtype_matrix where NumPy returns "double" and test_add_dtype_error which attempts to use "f8" regardless target device capabilities.

UPD: I pushed fixes for this and added tests to increase coverage.

Also ensure that test_add_order exercises non-same dtypes to improve coverage.

github-actions · 2023-05-20T16:58:50Z

Array API standard conformance tests for dpctl=0.14.3dev1=py310h76be34b_111 ran successfully.
Passed: 228
Failed: 772
Skipped: 116

github-actions · 2023-05-20T17:07:19Z

Array API standard conformance tests for dpctl=0.14.3dev1=py310h76be34b_112 ran successfully.
Passed: 227
Failed: 773
Skipped: 116

oleksandr-pavlyk · 2023-05-20T17:39:32Z

        assert dpt.abs(X).dtype == type_map[arg_dt]
+
+        out = dpt.empty_like(X, dtype=type_map[arg_dt])
+        dpt.abs(X, out)


I think out should not be allowed to be a positional argument in array API. Could you please change it to a keyword argument in the test suite?

github-actions · 2023-05-20T19:12:57Z

Array API standard conformance tests for dpctl=0.14.3dev1=py310h76be34b_113 ran successfully.
Passed: 228
Failed: 772
Skipped: 116

oleksandr-pavlyk

Thank you @vtavana ! Please merge when ready

github-actions · 2023-05-21T00:00:19Z

Deleted rendered PR docs from intelpython.github.com/dpctl, latest should be updated shortly. 🤞

vtavana self-assigned this May 19, 2023

vtavana changed the base branch from master to feature/elementwise-functions May 19, 2023 01:58

vtavana requested a review from oleksandr-pavlyk May 19, 2023 02:25

oleksandr-pavlyk suggested changes May 19, 2023

View reviewed changes

Comment thread dpctl/tests/elementwise/test_add.py Outdated

Comment thread dpctl/tests/elementwise/test_add.py Outdated

out keyword for elementwise functions

aef6de3

vtavana force-pushed the feature/elementwise-functions-out-keyword branch from 8d1f68b to f774228 Compare May 19, 2023 03:20

update the tests

21e1415

vtavana force-pushed the feature/elementwise-functions-out-keyword branch from f774228 to 21e1415 Compare May 19, 2023 15:50

add new tests to improve coverage

121f819

oleksandr-pavlyk added 2 commits May 20, 2023 11:11

Fixed tests to run on Iris Xe

05ae945

Also ensure that test_add_order exercises non-same dtypes to improve coverage.

Added a test for broadcasting error

d691846

oleksandr-pavlyk self-requested a review May 20, 2023 17:37

oleksandr-pavlyk reviewed May 20, 2023

View reviewed changes

use keywrod argument

f3d5519

oleksandr-pavlyk approved these changes May 20, 2023

View reviewed changes

vtavana merged commit cc75cd2 into feature/elementwise-functions May 20, 2023

vtavana deleted the feature/elementwise-functions-out-keyword branch May 21, 2023 00:11

Conversation

vtavana commented May 19, 2023 • edited by oleksandr-pavlyk Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented May 19, 2023

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented May 19, 2023

Uh oh!

github-actions Bot commented May 19, 2023

Uh oh!

oleksandr-pavlyk commented May 19, 2023

Uh oh!

github-actions Bot commented May 19, 2023

Uh oh!

oleksandr-pavlyk commented May 19, 2023

Uh oh!

vtavana commented May 20, 2023

Uh oh!

github-actions Bot commented May 20, 2023

Uh oh!

oleksandr-pavlyk commented May 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented May 20, 2023

Uh oh!

github-actions Bot commented May 20, 2023

Uh oh!

oleksandr-pavlyk May 20, 2023

Choose a reason for hiding this comment

Uh oh!

vtavana May 20, 2023

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented May 20, 2023

Uh oh!

oleksandr-pavlyk left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented May 21, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

vtavana commented May 19, 2023 •

edited by oleksandr-pavlyk

Loading

oleksandr-pavlyk commented May 20, 2023 •

edited

Loading