Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Feature/support sycl queue in array constructor functions #1032

Merged
merged 25 commits into from
May 31, 2023
Merged

Feature/support sycl queue in array constructor functions #1032

merged 25 commits into from
May 31, 2023

Conversation

diptorupd
Copy link
Contributor

@diptorupd diptorupd commented May 8, 2023 •
edited
Loading

Enables the sycl_queue keyword in the various dpnp array constructors (empty, empty_like, full, full_like, zeros, zeros_like, ones, ones_like). The passed in dpctl.SyclQueue or the one infered during type inference of the dpnp.ndarray is used to allocate memory.

The use of filter string in memory allocation is removed.

Improvements to the existing unit test cases and new ones.

  • 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?
  • If this PR is a work in progress, are you filing the PR as a draft?

@diptorupd diptorupd requested a review from chudur-budur as a code owner May 8, 2023 03:55
@diptorupd diptorupd mentioned this pull request May 8, 2023
Closed
5 tasks
chudur-budur
chudur-budur reviewed May 8, 2023
View reviewed changes
Copy link
Contributor

@chudur-budur chudur-budur left a comment

Choose a reason for hiding this comment

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

Let's keep things simpler in these test files. For each function, we do these:

  1. call dpnp function with default argument
  2. call dpnp function with queue and pytest parameterized arguments
  3. call dpnp function with device and pytest parameteried arguments
  4. call dpnp with both queue and device (exception check)

numba_dpex/tests/types/USMNdArray/test_array_creation_errors.py Outdated Show resolved Hide resolved
numba_dpex/tests/dpjit_tests/dpnp/test_dpnp_empty.py Show resolved Hide resolved
numba_dpex/tests/dpjit_tests/dpnp/test_dpnp_empty_like.py Outdated Show resolved Hide resolved
numba_dpex/tests/dpjit_tests/dpnp/test_dpnp_empty_like.py Outdated Show resolved Hide resolved
numba_dpex/tests/dpjit_tests/dpnp/test_dpnp_empty.py Outdated Show resolved Hide resolved
numba_dpex/tests/dpjit_tests/dpnp/test_dpnp_empty.py Outdated Show resolved Hide resolved
numba_dpex/tests/dpjit_tests/dpnp/test_dpnp_empty_like.py Outdated Show resolved Hide resolved
numba_dpex/tests/dpjit_tests/dpnp/test_dpnp_empty_like.py Outdated Show resolved Hide resolved
numba_dpex/tests/dpjit_tests/dpnp/test_dpnp_empty.py Outdated Show resolved Hide resolved
chudur-budur
chudur-budur reviewed May 8, 2023
View reviewed changes
Copy link
Contributor

@chudur-budur chudur-budur left a comment

Choose a reason for hiding this comment

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

Let's keep things simpler in these test files. For each function, we do these:

  1. call dpnp function with default argument
  2. call dpnp function with queue and pytest parameterized arguments
  3. call dpnp function with device and pytest parameteried arguments
  4. call dpnp with both queue and device (exception check)

@diptorupd diptorupd force-pushed the feature/support_sycl_queue_in_array_ctors branch from 9722585 to f785652 Compare May 10, 2023 19:58
@chudur-budur chudur-budur force-pushed the feature/support_sycl_queue_in_array_ctors branch from f785652 to 9c61948 Compare May 10, 2023 20:06
@diptorupd diptorupd force-pushed the feature/support_sycl_queue_in_array_ctors branch 3 times, most recently from a5726e3 to 187b7a9 Compare May 10, 2023 22:00
@chudur-budur chudur-budur force-pushed the feature/support_sycl_queue_in_array_ctors branch 2 times, most recently from 6414ebc to 60d8783 Compare May 10, 2023 23:28
@diptorupd diptorupd force-pushed the feature/support_sycl_queue_in_array_ctors branch from 60d8783 to 9fc29f0 Compare May 15, 2023 17:04
@chudur-budur chudur-budur force-pushed the feature/support_sycl_queue_in_array_ctors branch from 1801d4d to 65dcd1c Compare May 16, 2023 17:10
@chudur-budur chudur-budur force-pushed the feature/support_sycl_queue_in_array_ctors branch 6 times, most recently from a39bc5d to 43470c7 Compare May 18, 2023 18:28
@chudur-budur chudur-budur mentioned this pull request May 20, 2023
Closed
5 tasks
@chudur-budur chudur-budur force-pushed the feature/support_sycl_queue_in_array_ctors branch 4 times, most recently from f53e8a1 to d7cd285 Compare May 24, 2023 17:35
@chudur-budur
Copy link
Contributor

I think it's ready for merge. @diptorupd

@chudur-budur chudur-budur force-pushed the feature/support_sycl_queue_in_array_ctors branch 2 times, most recently from 7ed5b04 to e9c0834 Compare May 25, 2023 07:41
@chudur-budur chudur-budur force-pushed the feature/support_sycl_queue_in_array_ctors branch from c923d92 to 11d44e0 Compare May 26, 2023 21:08
khaled and others added 25 commits May 30, 2023 21:52
Enables the use of dpctl.SyclQueue for array creations
8a9953f 
    - Enables using the queue keyword for dpnp array constructors.
    - Uses the DpctlSyclQueueRef for memory allocations and remove
      using a filter string to first create a queue.
    - Unit tests.
Copy queue_ref before use in allocator.
2ef0f25
Refactor to move new code out of _patches.
70c2e9d
Remove memory leak due to extra queue copy.
3ed052e 
    - Removes a queue copy during NRTExternalAllocator as the
      ownership of the passed in queue belongs to the
      NRTExternalAllocator and the extra copy introduced a memory
      leak. It is the caller's responsibility to do any queue
      copy if needed.
    - Reverts all superfluous naming changes to reduce the diff.
    - Fixes few docstrings that needed to be updated.
Update unit tests for dpnp array constructors
4a0dd23
Update unit tests for dpnp.empty and dpnp.empty_like
7c62c0b
Improve unit tests for empty and empty_like.
30adfb2
Fix test case.
72b094b
Refactored all dpnp functions overload
ab9a783
Remove driver.py and move tests/types tests into tests/core/types
e849cf1 
Refactored tests/core/types/USMNdArray/test_array_creation_errors.py, since we got rid of filter_string and USMNdArray allocates int64 by default, not float64, like they do in numpy and dpnp

Need to use dpctl._sycl_queue_manager
Set to xfail due to github issue 1046
5bd8bfd
Fix grammar in error messages
9985518 
Revert to original coverage.yml
Merge 1047, fix caching of DpctlSyclQueue issues
6ec956b
Move dpctl.SyclQueue() calls inside try:
2f6802b
Fix dpctl_types for unique_id and proper type check for segfault
3cc873b
Skipping test_queue_ref_attr to avoid segfault for now
00f2681
Skipping test_dpjit_reduction.py's test_dpjit_array_arg_types_add1 to…
f773438 
… avoid segfault
Update numba_dpex/__init__.py from main
1969ec4
Skip test_dpjit_array_arg_types_add1 in test_dpjit_reduction.py
2936110
Remove is_fill_value_float flag
4fea02c
Fix windows compilation warnings
7b5c6a3
USMNdArray returns float64 array by default
c15d292
xfailing dtype mismatch on windows
80456a0
skip dpnp.full() for large ints on windows
58286fc
Remove unused imports
50dcaec 
Minor typo fix
@diptorupd diptorupd force-pushed the feature/support_sycl_queue_in_array_ctors branch from 11d44e0 to 50dcaec Compare May 31, 2023 02:52
@diptorupd diptorupd merged commit f54cdc1 into main May 31, 2023
@diptorupd diptorupd deleted the feature/support_sycl_queue_in_array_ctors branch May 31, 2023 03:49
github-actions bot added a commit that referenced this pull request May 31, 2023
@github-actions
Merge pull request #1032 from IntelPython/feature/support_sycl_queue_…
6c83ee1 
…in_array_ctors

Feature/support sycl queue in array constructor functions f54cdc1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chudur-budur chudur-budur chudur-budur left review comments

@mingjie-intel mingjie-intel Awaiting requested review from mingjie-intel

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@diptorupd @chudur-budur