Skip to content

Implement dpnp.compress and dpnp_array.compress method #2177

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 21 commits into from
Nov 26, 2024

Conversation

ndgrigorian
Copy link
Collaborator

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?

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

@antonwolfy antonwolfy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

@antonwolfy antonwolfy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @ndgrigorian

@ndgrigorian
Copy link
Collaborator Author

@antonwolfy CI is passed, please merge at your convenience

@antonwolfy antonwolfy merged commit 367e74e into IntelPython:master Nov 26, 2024
47 of 48 checks passed
github-actions bot added a commit that referenced this pull request Nov 26, 2024
* 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
@ndgrigorian ndgrigorian deleted the dpnp-compress-impl branch November 26, 2024 19:14
vtavana pushed a commit that referenced this pull request Dec 2, 2024
* 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`
vtavana pushed a commit that referenced this pull request Dec 2, 2024
* 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`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants