Skip to content

Refactor dpnp includes and add extension example#2941

Open
ndgrigorian wants to merge 12 commits into
masterfrom
add-libtensor-pybind11-example
Open

Refactor dpnp includes and add extension example#2941
ndgrigorian wants to merge 12 commits into
masterfrom
add-libtensor-pybind11-example

Conversation

@ndgrigorian

Copy link
Copy Markdown
Collaborator

This PR proposes a refactor of dpnp's includes and the introduction of an example extension, to be tested to make sure examples using dpnp and dpnp.tensor can be written.

The usm_ndarray_constants.h header and dpnp4pybind11.hpp are moved to dpnp/include, removing a relative include and aligning with previous dpctl behavior.

Also introduces dpnp-config.cmake, which enables find_package(Dpnp) out of the box, which is installed with dpnp, and --cmakedir command line option, which leads to the CMake config.

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

Enables find_package(Dpnp) out of the box
move dpnp4pybind11.hpp and usm_ndarray_constants.h into a top-level include directory, similar to original dpctl layout
serves to test dpnp cmake integration
@github-actions

github-actions Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

View rendered docs @ https://intelpython.github.io/dpnp/pull/2941/index.html

@coveralls

coveralls commented Jun 5, 2026

Copy link
Copy Markdown
Collaborator

Coverage Status

coverage: 78.101% (-0.02%) from 78.116% — add-libtensor-pybind11-example into master

@github-actions

github-actions Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Array API standard conformance tests for dpnp=0.21.0dev0=py313h509198e_82 ran successfully.
Passed: 1356
Failed: 4
Skipped: 16

@ndgrigorian ndgrigorian force-pushed the add-libtensor-pybind11-example branch from 622aa32 to c735f53 Compare June 5, 2026 08:30
Comment thread examples/pybind11/use_dpnp_array/tests/test_use_dpnp_array.py Outdated
Comment thread examples/pybind11/use_dpnp_array/use_dpnp_array/__init__.py Outdated
Comment thread examples/pybind11/use_dpnp_array/CMakeLists.txt Outdated
Comment thread examples/pybind11/use_dpnp_array/setup.py Outdated
Comment thread examples/pybind11/use_dpnp_array/setup.py Outdated
Comment thread dpnp/__main__.py Outdated
Comment thread cmake/dpnp-config.cmake Outdated
Comment thread examples/pybind11/use_dpnp_array/CMakeLists.txt Outdated
@ndgrigorian ndgrigorian force-pushed the add-libtensor-pybind11-example branch from cae3b56 to 020e5ea Compare June 5, 2026 22:58
@ndgrigorian ndgrigorian marked this pull request as ready for review June 6, 2026 00:32
@ndgrigorian ndgrigorian requested a review from antonwolfy as a code owner June 6, 2026 00:32

@vlad-perevezentsev vlad-perevezentsev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM
Thank you @ndgrigorian
A few minor comments below:

Comment thread .gitignore Outdated
Comment thread cmake/dpnp-config.cmake Outdated

@vlad-perevezentsev vlad-perevezentsev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Great job!
No more comments from my side
LGTM, thank you @ndgrigorian

@antonwolfy antonwolfy added this to the 0.21.0 release milestone Jun 11, 2026
Comment thread dpnp/__main__.py
help="Path to dpnp libtensor include directory.",
)
parser.add_argument(
"--cmakedir",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Would it make sense to test --cmakedir / Dpnp_ROOT in the example also?

Comment thread cmake/dpnp-config.cmake
#

if(NOT Dpnp_FOUND)
find_package(Python 3.10 REQUIRED COMPONENTS Interpreter Development.Module)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Would it make sense to have upper limit also?

Suggested change
find_package(Python 3.10 REQUIRED COMPONENTS Interpreter Development.Module)
find_package(Python 3.10...<3.15 REQUIRED COMPONENTS Interpreter Development.Module)

Comment thread .gitignore
dpnp/**/*.pyd
*~
core
*.so

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We have a pattern with ...*so already above. Do we need to be more specific here and to ignore only ones generated in example folder?


- name: Install example requirements
run: |
mamba install pytest dpcpp_linux-64">=2026.0.0" cmake ninja scikit-build ${{ env.TEST_CHANNELS }}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

As an option we can add a dedicated env file to dpnp/enviroments folder and install all the requirements in Setup miniconda step

Comment thread cmake/dpnp-config.cmake
Dpnp_INCLUDE_DIR
dpnp4pybind11.hpp
PATHS "${_dpnp_include_dir}" "${Python_INCLUDE_DIRS}"
PATH_SUFFIXES dpnp/include

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we need to use NO_DEFAULT_PATH here ?

Comment thread CMakeLists.txt
Comment on lines +331 to +335
install(
FILES ${CMAKE_SOURCE_DIR}/cmake/dpnp-config.cmake
DESTINATION dpnp/resources/cmake
)
install(FILES ${CMAKE_SOURCE_DIR}/cmake/dpnp-config.cmake DESTINATION lib/cmake/dpnp)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We can reduce duplication:

Suggested change
install(
FILES ${CMAKE_SOURCE_DIR}/cmake/dpnp-config.cmake
DESTINATION dpnp/resources/cmake
)
install(FILES ${CMAKE_SOURCE_DIR}/cmake/dpnp-config.cmake DESTINATION lib/cmake/dpnp)
foreach(_dest IN ITEMS dpnp/resources/cmake lib/cmake/dpnp)
install(FILES ${CMAKE_SOURCE_DIR}/cmake/dpnp-config.cmake DESTINATION ${_dest})
endforeach()

Comment thread CMakeLists.txt

# install dpnp4pybind11.hpp and usm_ndarray_constants.h into include folder
install(
DIRECTORY ${CMAKE_SOURCE_DIR}/dpnp/include/

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It actually means the recurses into ALL subdirs. But there is another rule at dpnp/CMakeLists.txt:147-156 which installs the Cython-generated headers from the build.
Thus the rule will grab dpnp/tensor/_usmarray.h and _usmarray_api.h and install them to <prefix>/dpnp/include/dpnp/tensor. Exactly the same as rules at dpnp/CMakeLists.txt:147-156 do.

So the same files might be written twice to the same location.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We might need to specify explicitly:

install(FILES
    ${CMAKE_SOURCE_DIR}/dpnp/include/dpnp4pybind11.hpp
    ${CMAKE_SOURCE_DIR}/dpnp/include/usm_ndarray_constants.h
    DESTINATION dpnp/include)

or to add PATTERN "dpnp/tensor" EXCLUDE to the end of install command.

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.

4 participants