Skip to content

Expose LocalAccessor as kernel argument type #1991

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
Merged
merged 11 commits into from
Mar 11, 2025
Merged

Expose LocalAccessor as kernel argument type #1991

merged 11 commits into from
Mar 11, 2025

Conversation

oleksandr-pavlyk
Copy link
Contributor

@oleksandr-pavlyk oleksandr-pavlyk commented Feb 10, 2025 •
edited by vtavana
Loading

This PR adds _md_local_accessor Cython struct in _backend.pxd that ties to MDLocalAccessor struct defined in libsyclinterface, and which is to be used to indicate local_accessor kernel argument type (see test file in syclinterface library).

_sycl_queue.pyx then defines LocalAccessor type, constructible using LocalAccessor(ndim, typestr, dim0, dim1, dim2) constuctor. The ndim argument represents accessor dimensionality (only values 1, 2, or 3 are supported). The typestr can be "i1", "u1", "i2", "u2", "i4", "u4", "i8", "u8", "f4", or "f8".

  • 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?
  • Have you added documentation for your changes, if necessary?
  • Have you added your changes to the changelog?
  • If this PR is a work in progress, are you opening the PR as a draft?

@github-actions GitHub Actions
Copy link

github-actions bot commented Feb 10, 2025 •
edited
Loading

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

@github-actions GitHub Actions
Copy link

Array API standard conformance tests for dpctl=0.19.0dev0=py310h93fe807_518 ran successfully.
Passed: 894
Failed: 2
Skipped: 118

@sommerlukas sommerlukas mentioned this pull request Feb 12, 2025
Merged
7 tasks
ndgrigorian
ndgrigorian reviewed Feb 14, 2025
View reviewed changes
@github-actions GitHub Actions
Copy link

Array API standard conformance tests for dpctl= ran successfully.
Passed: 894
Failed: 2
Skipped: 118

@coveralls
Copy link
Collaborator

coveralls commented Feb 17, 2025 •
edited
Loading

Coverage Status

coverage: 87.652% (-0.2%) from 87.855%
when pulling 1d3453e on add-md-local-accessor
into 96da90b on master.

@github-actions GitHub Actions
Copy link

Array API standard conformance tests for dpctl=0.19.0dev0=py310h93fe807_540 ran successfully.
Passed: 894
Failed: 2
Skipped: 118

sommerlukas
sommerlukas reviewed Feb 24, 2025
View reviewed changes
@ndgrigorian ndgrigorian force-pushed the add-md-local-accessor branch from ff43ac7 to e1e4b00 Compare February 27, 2025 08:38
@github-actions GitHub Actions
Copy link

Array API standard conformance tests for dpctl=0.20.0dev0=py310h93fe807_11 ran successfully.
Passed: 894
Failed: 2
Skipped: 118

@ndgrigorian ndgrigorian force-pushed the add-md-local-accessor branch from e1e4b00 to 949571b Compare February 27, 2025 19:14
@github-actions GitHub Actions
Copy link

Array API standard conformance tests for dpctl=0.20.0dev0=py310h93fe807_11 ran successfully.
Passed: 893
Failed: 3
Skipped: 118

@ndgrigorian ndgrigorian force-pushed the add-md-local-accessor branch from 949571b to f35e483 Compare February 27, 2025 22:22
@github-actions GitHub Actions
Copy link

Array API standard conformance tests for dpctl=0.20.0dev0=py310h93fe807_11 ran successfully.
Passed: 896
Failed: 1
Skipped: 125

sommerlukas
sommerlukas reviewed Feb 28, 2025
View reviewed changes
oleksandr-pavlyk and others added 9 commits March 7, 2025 13:18
@oleksandr-pavlyk @ndgrigorian
Export _md_local_accessor Cython struct, mapped to C struct MDLocalAc…
b6d639a 
…cessor
@oleksandr-pavlyk @ndgrigorian
Define LocalAccessor type to use to specify local accessor kernel arg…
fd01ba5 
…uments

LocalAccessor(ndim, elemental_type_str, dim0, dim1, dim2)

The elemental type can be one of the following:
 "i1", "u1", "i2", "u2", "i4", "u4", "i8", "u8", "f4", "f8"
@oleksandr-pavlyk @ndgrigorian
Add Python tests for kernel submission with LocalAccessor
c0019d5
@ndgrigorian
Fix typo in _md_local_accessor error message
c849993
@ndgrigorian
type->dtype in LocalAccessor initialization
1f94e7f 
Fixes conflict with built-in `type`
@ndgrigorian
Add docstrings to LocalAccessor
76fdd08 
Also improve messages in errors
@ndgrigorian
Add a comment documenting the origin of spirv files in test_sycl_kern…
5c07e44 
…el_submit.py
@ndgrigorian
Add LocalAccessor to dpctl namespace
6cdb80c
@ndgrigorian
Fix __repr__ in LocalAccessor
5fc86c6
@ndgrigorian ndgrigorian force-pushed the add-md-local-accessor branch from f35e483 to 5fc86c6 Compare March 7, 2025 21:22
@github-actions GitHub Actions
Copy link

github-actions bot commented Mar 7, 2025

Array API standard conformance tests for dpctl=0.20.0dev0=py310h93fe807_41 ran successfully.
Passed: 895
Failed: 2
Skipped: 126

@ndgrigorian
LocalAccessor takes a sequence of non-negative integers instead of se…
b5887df 
…parate integer arguments

Update test for LocalAccessor kernel submission
@ndgrigorian ndgrigorian requested a review from sommerlukas March 8, 2025 08:24
@github-actions GitHub Actions
Copy link

github-actions bot commented Mar 8, 2025

Array API standard conformance tests for dpctl=0.20.0dev0=py310h93fe807_42 ran successfully.
Passed: 894
Failed: 3
Skipped: 126

ndgrigorian
ndgrigorian previously approved these changes Mar 10, 2025
View reviewed changes
vtavana
vtavana previously approved these changes Mar 11, 2025
View reviewed changes
Copy link
Collaborator

@vtavana vtavana 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, for finalizing it!

@ndgrigorian
Address review comments on test_submit_local_accessor_arg
1d3453e 
Also slips in a fix to the shape of the local_accessor
@ndgrigorian ndgrigorian dismissed stale reviews from vtavana and themself via 1d3453e March 11, 2025 00:32
@ndgrigorian ndgrigorian requested a review from vtavana March 11, 2025 00:32
@github-actions GitHub Actions
Copy link

Array API standard conformance tests for dpctl=0.20.0dev0=py310h93fe807_45 ran successfully.
Passed: 895
Failed: 2
Skipped: 126

@ndgrigorian ndgrigorian merged commit c3e6952 into master Mar 11, 2025
73 of 74 checks passed
@ndgrigorian ndgrigorian deleted the add-md-local-accessor branch March 11, 2025 05:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ndgrigorian ndgrigorian ndgrigorian left review comments

@sommerlukas sommerlukas sommerlukas approved these changes

@vtavana vtavana vtavana approved these changes

@antonwolfy antonwolfy Awaiting requested review from antonwolfy antonwolfy is a code owner

@vlad-perevezentsev vlad-perevezentsev Awaiting requested review from vlad-perevezentsev vlad-perevezentsev is a code owner

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

Successfully merging this pull request may close these issues.
5 participants
@oleksandr-pavlyk @coveralls @sommerlukas @ndgrigorian @vtavana