Skip to content

Cleanup/tensor #941

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 24 commits into from
Oct 18, 2022
Merged

Cleanup/tensor #941

merged 24 commits into from
Oct 18, 2022

Conversation

oleksandr-pavlyk
Copy link
Contributor

This PR is the composition of #925 #931 #932 #933 #934

This PR supersedes #923.

  • Have you provided a meaningful PR description?
  • Have you tested your changes locally for CPU and GPU devices?
  • Have you made sure that new changes do not introduce compiler warnings?

This is to avoid multiple definitions compilation error when using
multiple translation units.
Moved implementation of kernels out to dedicated header files.
Use -O1 when compiling tensor_py for now to work around suspected issue
with loading of C-API functions.
…es`.

The class is already singleton. Instead create a local variable at
each use site. This local variable is going to be a constant reference
to the singleton.
[cleanup/tensor, part 2] Modularized tests for contiguity, retrieval of PyUSMArrayObject* and removed use of a global variable.
The class implements a singleton pattern that populates global
variables exported by Cython CAPI functions via call to import_dpctl()
and then populates internal struct members with pointers populated
by Cython.

This loads both Cython 'public api' functions, types, and integer values.

dpctl::tensor::usm_ndarray and dpctl::memory::usm_memory classes, as
well as type-casters for dpctl.SyclQueue <-> sycl::queue,
dpctl.SyclDevice <-> sycl::device, etc. use dpctl_capi.

The singleton patterns helps avoid static initialization order fiasco
problem when using multiple translation units.

Single dpctl_capi loads typenum values, the
dpctl::tensor::detail::usm_ndarray_types struct no longer needs to be
a singleton, and becomes an ordinary struct with a static method.

tensor_py.cpp was changed to reflect that.
[cleanup/tensor, step 3] Implement dpctl_capi loader in dpctl4pybind11 header file
This is avoid using py::value_error and py::ssize_t and py::type_error
in there explicitly.

Use in tensor_py.cpp adjusted
[cleanup/tensor, part 4] Made contract_iter and contract_iter2 functions templated to avoid needing to include pybind11 headers
…ed file

Also moved routine for simplification of iteration space into dedicated file
@github-actions
Copy link

@coveralls
Copy link
Collaborator

Coverage Status

Coverage remained the same at 82.136% when pulling 30223a2 on cleanup/tensor into ab46d49 on master.

@github-actions
Copy link

Array API standard conformance tests for dpctl=0.14.0dev0=py310h8c27c75_192 ran successfully.
Passed: 33
Failed: 801
Skipped: 3138

@oleksandr-pavlyk
Copy link
Contributor Author

This individual merges were reviewed, and CI is green, merging.

@oleksandr-pavlyk oleksandr-pavlyk merged commit 59980a2 into master Oct 18, 2022
@oleksandr-pavlyk oleksandr-pavlyk deleted the cleanup/tensor branch October 18, 2022 13:06
@github-actions
Copy link

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

@github-actions
Copy link

Array API standard conformance tests for dpctl=0.14.0dev0=py310h8c27c75_192 ran successfully.
Passed: 33
Failed: 801
Skipped: 3138

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