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

Add support of CV-qualifiers in is_complex<T> helper #1900

Merged
merged 4 commits into from
Nov 19, 2024
Merged

Add support of CV-qualifiers in is_complex<T> helper #1900

merged 4 commits into from
Nov 19, 2024

Conversation

antonwolfy
Copy link
Collaborator

@antonwolfy antonwolfy commented Nov 19, 2024 •
edited
Loading

Current implementation of is_complex<T> is limited in dpctl tensor headers. It doesn't consider CV-qualifiers for a type.
While STL wrappers of DPC++ has more complete implementation.

The PR proposes to align with the STL wrappers and to add support of CV-qualifiers in is_complex<T> implementation. And also it suggests to add is_complex_v<T> helper.

The PR would help to simplify implementation of histogramdd extension in DPNP.

  • 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?

@antonwolfy antonwolfy self-assigned this Nov 19, 2024
@antonwolfy antonwolfy mentioned this pull request Nov 19, 2024
Merged
@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 19, 2024 •
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=py310hdf72452_239 ran successfully.
Passed: 894
Failed: 1
Skipped: 119

@coveralls
Copy link
Collaborator

coveralls commented Nov 19, 2024 •
edited
Loading

Coverage Status

Changes unknown
when pulling 2e39e98 on align-is_complex-with-stl-wrappers
into ** on master**.

@github-actions GitHub Actions
Copy link

Array API standard conformance tests for dpctl=0.19.0dev0=py310hdf72452_241 ran successfully.
Passed: 895
Failed: 0
Skipped: 119

@github-actions GitHub Actions
Copy link

Array API standard conformance tests for dpctl=0.19.0dev0=py310hdf72452_242 ran successfully.
Passed: 894
Failed: 1
Skipped: 119

@antonwolfy antonwolfy marked this pull request as ready for review November 19, 2024 14:00
@antonwolfy
Copy link
Collaborator Author

Seems the issue in internal CI doesn't relate to that PR.

@oleksandr-pavlyk
Copy link
Collaborator

I understand the need to support CV qualified types, but I am not sure how to feel about sycl::complex<sycl::half> since dpctl does not support this type.

How does supporting complex<half> help dpnp?

@antonwolfy
Copy link
Collaborator Author

I understand the need to support CV qualified types, but I am not sure how to feel about sycl::complex<sycl::half> since dpctl does not support this type.

How does supporting complex<half> help dpnp?

Only support of CV-qualifiers is required, I will remove sycl::complex<sycl::half>.

@antonwolfy antonwolfy changed the title Add support of cv-qualifiers and sycl::half type in is_complex<T> implementation Add support of CV-qualifiers in is_complex<T> helper Nov 19, 2024
@antonwolfy antonwolfy force-pushed the align-is_complex-with-stl-wrappers branch from 9def7d9 to febc5dc Compare November 19, 2024 14:28
@github-actions GitHub Actions
Copy link

Array API standard conformance tests for dpctl=0.19.0dev0=py310hdf72452_243 ran successfully.
Passed: 895
Failed: 0
Skipped: 119

@github-actions GitHub Actions
Copy link

Array API standard conformance tests for dpctl=0.19.0dev0=py310hdf72452_244 ran successfully.
Passed: 894
Failed: 1
Skipped: 119

@github-actions GitHub Actions
Copy link

Array API standard conformance tests for dpctl=0.19.0dev0=py310hdf72452_244 ran successfully.
Passed: 895
Failed: 0
Skipped: 119

@ndgrigorian
Copy link
Collaborator

type_utils.hpp doesn't seem to include type_traits, either.

Do you mind adding that in this PR @antonwolfy ?

@antonwolfy
Copy link
Collaborator Author

type_utils.hpp doesn't seem to include type_traits, either.

Do you mind adding that in this PR @antonwolfy ?

Sure, included.

ndgrigorian
ndgrigorian approved these changes Nov 19, 2024
View reviewed changes
Copy link
Collaborator

@ndgrigorian ndgrigorian left a comment

Choose a reason for hiding this comment

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

LGTM!

oleksandr-pavlyk
oleksandr-pavlyk approved these changes Nov 19, 2024
View reviewed changes
Copy link
Collaborator

@oleksandr-pavlyk oleksandr-pavlyk left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thank you @antonwolfy

@github-actions GitHub Actions
Copy link

Array API standard conformance tests for dpctl=0.19.0dev0=py310hdf72452_245 ran successfully.
Passed: 894
Failed: 1
Skipped: 119

@antonwolfy antonwolfy merged commit d6b6aa5 into master Nov 19, 2024
36 of 46 checks passed
@antonwolfy antonwolfy deleted the align-is_complex-with-stl-wrappers branch November 19, 2024 20:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ndgrigorian ndgrigorian ndgrigorian approved these changes

@oleksandr-pavlyk oleksandr-pavlyk oleksandr-pavlyk approved these changes

Assignees

@antonwolfy antonwolfy

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@antonwolfy @coveralls @oleksandr-pavlyk @ndgrigorian