Implement `dpnp.compress` and `dpnp_array.compress` method by ndgrigorian · Pull Request #2177 · IntelPython/dpnp

ndgrigorian · 2024-11-18T04:30:46Z

This PR proposes an implementation for dpnp.compress using dpctl.tensor kernels.

nonzero is used to accumulate the condition into 1d array of indices, which are then used to take the appropriate slices of the array. When axis is None, the operand array is flattened.

The coupled dpnp_array.compress method is implemented through a simple call to the compress function,

Have you provided a meaningful PR description?
Have you added a test, reproducer or referred to 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 filing the PR as a draft?

dpnp/dpnp_iface_indexing.py

dpnp/tests/test_indexing.py

Also disable checks for protected access, as `compress` uses dpctl.tensor private functions

Also removes an unnecessary check per PR review

Also fix a typo when `condition` is not an array

Also tweaks to docstring

Should slightly improve efficiency by escaping an additional copy where `out` is not `None` and flattening of indices

dpnp/dpnp_iface_indexing.py

dpnp/tests/test_usm_type.py

dpnp/dpnp_array.py

…to `compress`

antonwolfy

Thank you @ndgrigorian. I have only a minor comment, but in overall LGTM!

dpnp/tests/test_usm_type.py

antonwolfy

Thank you @ndgrigorian

ndgrigorian · 2024-11-26T18:24:29Z

@antonwolfy CI is passed, please merge at your convenience

@antonwolfy

* Implement dpnp.compress * Add `dpnp_array.compress` method * Break up `compress` to satisfy pylint Also disable checks for protected access, as `compress` uses dpctl.tensor private functions * Unmute third-party tests for `compress` * Use `get_usm_allocations` in `compress` * Fix bug where `out` in `compress` is dpnp_array Also removes an unnecessary check per PR review * Apply comments per PR review by @antonwolfy Also fix a typo when `condition` is not an array * Remove branching when `condition` is an array Also tweaks to docstring * Add tests for `compress` * Re-use `_take_index` for `dpnp.take` Should slightly improve efficiency by escaping an additional copy where `out` is not `None` and flattening of indices * Change error for incorrect out array dtype to `TypeError` * Move compress tests into a TestCompress class * Use NumPy in compress tests * Add `no_none=True` to `test_compress_condition_all_dtypes` * Add USM type and SYCL queue tests for `compress` * More tests for compress added * Docstring change per PR review * Integrate test for compute follows data in compress into test_2in_1out * Add test for `dpnp_array.compress` and add a test for strided inputs to `compress` * Refactor `test_compress` in test_usm_type.py into `test_2in_1out` 367e74e

@antonwolfy

* Implement dpnp.compress * Add `dpnp_array.compress` method * Break up `compress` to satisfy pylint Also disable checks for protected access, as `compress` uses dpctl.tensor private functions * Unmute third-party tests for `compress` * Use `get_usm_allocations` in `compress` * Fix bug where `out` in `compress` is dpnp_array Also removes an unnecessary check per PR review * Apply comments per PR review by @antonwolfy Also fix a typo when `condition` is not an array * Remove branching when `condition` is an array Also tweaks to docstring * Add tests for `compress` * Re-use `_take_index` for `dpnp.take` Should slightly improve efficiency by escaping an additional copy where `out` is not `None` and flattening of indices * Change error for incorrect out array dtype to `TypeError` * Move compress tests into a TestCompress class * Use NumPy in compress tests * Add `no_none=True` to `test_compress_condition_all_dtypes` * Add USM type and SYCL queue tests for `compress` * More tests for compress added * Docstring change per PR review * Integrate test for compute follows data in compress into test_2in_1out * Add test for `dpnp_array.compress` and add a test for strided inputs to `compress` * Refactor `test_compress` in test_usm_type.py into `test_2in_1out`

@antonwolfy

* Implement dpnp.compress * Add `dpnp_array.compress` method * Break up `compress` to satisfy pylint Also disable checks for protected access, as `compress` uses dpctl.tensor private functions * Unmute third-party tests for `compress` * Use `get_usm_allocations` in `compress` * Fix bug where `out` in `compress` is dpnp_array Also removes an unnecessary check per PR review * Apply comments per PR review by @antonwolfy Also fix a typo when `condition` is not an array * Remove branching when `condition` is an array Also tweaks to docstring * Add tests for `compress` * Re-use `_take_index` for `dpnp.take` Should slightly improve efficiency by escaping an additional copy where `out` is not `None` and flattening of indices * Change error for incorrect out array dtype to `TypeError` * Move compress tests into a TestCompress class * Use NumPy in compress tests * Add `no_none=True` to `test_compress_condition_all_dtypes` * Add USM type and SYCL queue tests for `compress` * More tests for compress added * Docstring change per PR review * Integrate test for compute follows data in compress into test_2in_1out * Add test for `dpnp_array.compress` and add a test for strided inputs to `compress` * Refactor `test_compress` in test_usm_type.py into `test_2in_1out`

ndgrigorian force-pushed the dpnp-compress-impl branch from cf9df6d to 9b05295 Compare November 18, 2024 07:46

antonwolfy reviewed Nov 18, 2024

View reviewed changes

dpnp/dpnp_iface_indexing.py Outdated Show resolved Hide resolved

ndgrigorian marked this pull request as ready for review November 18, 2024 22:15

ndgrigorian requested review from npolina4, vlad-perevezentsev and vtavana as code owners November 18, 2024 22:15

antonwolfy reviewed Nov 19, 2024

View reviewed changes

dpnp/tests/test_indexing.py Outdated Show resolved Hide resolved

antonwolfy reviewed Nov 19, 2024

View reviewed changes

dpnp/tests/test_indexing.py Outdated Show resolved Hide resolved

antonwolfy reviewed Nov 19, 2024

View reviewed changes

dpnp/tests/test_indexing.py Outdated Show resolved Hide resolved

antonwolfy reviewed Nov 19, 2024

View reviewed changes

dpnp/tests/test_indexing.py Outdated Show resolved Hide resolved

ndgrigorian added 16 commits November 23, 2024 15:06

Implement dpnp.compress

9641064

Add dpnp_array.compress method

466edd9

Break up compress to satisfy pylint

07304be

Also disable checks for protected access, as `compress` uses dpctl.tensor private functions

Unmute third-party tests for compress

e0aa410

Use get_usm_allocations in compress

e22b968

Fix bug where out in compress is dpnp_array

1f51311

Also removes an unnecessary check per PR review

Apply comments per PR review by @antonwolfy

fc85e73

Also fix a typo when `condition` is not an array

Remove branching when condition is an array

2381662

Also tweaks to docstring

Add tests for compress

49bfa0e

Re-use _take_index for dpnp.take

e53b84e

Should slightly improve efficiency by escaping an additional copy where `out` is not `None` and flattening of indices

Change error for incorrect out array dtype to TypeError

5818d10

Move compress tests into a TestCompress class

9a4430c

Use NumPy in compress tests

b4264a8

Add no_none=True to test_compress_condition_all_dtypes

81d8181

Add USM type and SYCL queue tests for compress

ec299bc

More tests for compress added

edace9e

ndgrigorian force-pushed the dpnp-compress-impl branch from f8886cd to edace9e Compare November 23, 2024 23:06

ndgrigorian requested a review from AlexanderKalistratov as a code owner November 23, 2024 23:06

ndgrigorian requested a review from antonwolfy November 25, 2024 03:27

antonwolfy reviewed Nov 25, 2024

View reviewed changes

dpnp/dpnp_iface_indexing.py Outdated Show resolved Hide resolved

dpnp/tests/test_usm_type.py Outdated Show resolved Hide resolved

dpnp/dpnp_array.py Show resolved Hide resolved

ndgrigorian added 3 commits November 25, 2024 12:13

Docstring change per PR review

cd6add4

Integrate test for compute follows data in compress into test_2in_1out

2ec457e

Add test for dpnp_array.compress and add a test for strided inputs …

108f76f

…to `compress`

ndgrigorian requested a review from antonwolfy November 25, 2024 23:42

antonwolfy approved these changes Nov 26, 2024

View reviewed changes

dpnp/tests/test_usm_type.py Outdated Show resolved Hide resolved

ndgrigorian and others added 2 commits November 26, 2024 09:16

Refactor test_compress in test_usm_type.py into test_2in_1out

4ca0e30

Merge branch 'master' into dpnp-compress-impl

0a8bfe3

ndgrigorian requested a review from antonwolfy November 26, 2024 17:27

antonwolfy approved these changes Nov 26, 2024

View reviewed changes

antonwolfy merged commit 367e74e into IntelPython:master Nov 26, 2024

ndgrigorian deleted the dpnp-compress-impl branch November 26, 2024 19:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement `dpnp.compress` and `dpnp_array.compress` method#2177

Implement `dpnp.compress` and `dpnp_array.compress` method#2177
antonwolfy merged 21 commits intoIntelPython:masterfrom
ndgrigorian:dpnp-compress-impl

ndgrigorian commented Nov 18, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

antonwolfy left a comment

Uh oh!

Uh oh!

antonwolfy left a comment

Uh oh!

ndgrigorian commented Nov 26, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

ndgrigorian commented Nov 18, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

antonwolfy left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

antonwolfy left a comment

Choose a reason for hiding this comment

Uh oh!

ndgrigorian commented Nov 26, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants