Add python bindings to read_tensor_data#32984
Add python bindings to read_tensor_data#32984olpipi wants to merge 4 commits intoopenvinotoolkit:masterfrom
Conversation
8e56854 to
56a5139
Compare
835410a to
f5c5a3b
Compare
src/bindings/python/tests/test_runtime/test_read_tensor_data.py
Outdated
Show resolved
Hide resolved
| regclass_Tensor(m); | ||
|
|
||
| m.def( | ||
| "read_tensor_data", |
There was a problem hiding this comment.
C++ API read_tensor_data is located in src/core/include/openvino/runtime/tensor.hpp
To not bloat pyopenvino.cpp, maybe it's better to move it into regclass Tensor?
@p-wysocki
e0ff46e to
dce9734
Compare
There was a problem hiding this comment.
Pull request overview
This pull request adds Python bindings for the read_tensor_data C++ function, enabling Python users to load tensor data directly from binary files with memory-mapping support.
Changes:
- Added
is_tensor_read_only()helper function in C++ to detect read-only tensors - Exposed
read_tensor_datafunction through Python bindings with proper parameter defaults - Modified
array_from_tensorto mark numpy arrays as read-only when the underlying tensor is read-only - Added comprehensive test coverage for the new Python API
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/inference/src/dev/make_tensor.cpp | Implements is_tensor_read_only() to identify read-only tensor views |
| src/inference/dev_api/openvino/runtime/make_tensor.hpp | Declares is_tensor_read_only() in the dev API with documentation |
| src/bindings/python/src/pyopenvino/pyopenvino.cpp | Adds Python binding for read_tensor_data with proper docstring |
| src/bindings/python/src/pyopenvino/core/common.cpp | Updates array_from_tensor to set numpy array writeable flag based on tensor read-only status |
| src/bindings/python/src/pyopenvino/CMakeLists.txt | Adds openvino::runtime::dev dependency to access dev API functions |
| src/bindings/python/src/openvino/init.py | Exports read_tensor_data in the main Python package |
| tools/ovc/openvino/init.py | Exports read_tensor_data in the OVC tool package |
| tools/benchmark_tool/openvino/init.py | Exports read_tensor_data in the benchmark tool package |
| src/bindings/python/tests/test_runtime/test_read_tensor_data.py | Comprehensive test suite covering various use cases and edge cases |
| bool is_tensor_read_only(const ov::Tensor& tensor) { | ||
| auto impl = get_tensor_impl(tensor); | ||
| if (std::dynamic_pointer_cast<ViewTensor>(impl._ptr)) { | ||
| return true; | ||
| } | ||
| return false; | ||
| } |
There was a problem hiding this comment.
[BLOCKER] The is_tensor_read_only function incorrectly identifies all ViewTensor subclasses as read-only, including writable ones. The current implementation checks for ViewTensor base class, but AllocatedTensor (which owns and manages writable memory) and StridedViewTensor (writable view with strides) also inherit from ViewTensor. This will cause normal allocated tensors and writable view tensors to be incorrectly marked as read-only in Python.
The function should specifically check for ReadOnlyViewTensor and ReadOnlyStridedViewTensor, not the base ViewTensor class. The correct implementation should be:
bool is_tensor_read_only(const ov::Tensor& tensor) {
auto impl = get_tensor_impl(tensor);
if (std::dynamic_pointer_cast<ReadOnlyViewTensor>(impl._ptr) ||
std::dynamic_pointer_cast<ReadOnlyStridedViewTensor>(impl._ptr)) {
return true;
}
return false;
}This is critical because it affects the Python bindings' behavior - regular allocated tensors would become read-only incorrectly, breaking existing functionality.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
src/bindings/python/tests/test_runtime/test_read_tensor_data.py:104
- This call formatting is inconsistent with the rest of the file (closing parenthesis on the same line as the last argument). Consider formatting it like the other multi-line calls (or running the project formatter) to keep the test file style consistent.
ov.read_tensor_data(
path,
element_type=ov.Type.f32,
shape=ov.PartialShape(list(shape)),
offset_in_bytes=1,
mmap=mmap)
| auto data_ptr = std::as_const(t).data(); | ||
| auto is_read_only = ov::is_tensor_read_only(t); | ||
|
|
||
| // Return the array as a view: | ||
| if (is_shared) { | ||
| py::array result; | ||
| if (ov_type.bitwidth() < Common::values::min_bitwidth) { | ||
| return py::array(dtype, t.get_byte_size(), t.data(), py::cast(t)); | ||
| result = py::array(dtype, t.get_byte_size(), data_ptr, py::cast(t)); | ||
| } else { | ||
| result = py::array(dtype, t.get_shape(), t.get_strides(), data_ptr, py::cast(t)); | ||
| } | ||
| if (is_read_only) { | ||
| // Mark array as read-only | ||
| result.attr("flags").attr("writeable") = false; | ||
| } | ||
| return py::array(dtype, t.get_shape(), t.get_strides(), t.data(), py::cast(t)); | ||
| return result; | ||
| } |
There was a problem hiding this comment.
This changes the Python-visible mutability semantics for any tensor detected as ViewTensor by is_tensor_read_only(). If writable view tensors exist, they will now surface as non-writeable NumPy arrays, which is a breaking behavior change for Python users. Once is_tensor_read_only() is corrected to match the intended 'const-host view' definition, this risk should be mitigated; alternatively, scope the read-only marking to tensors created by read_tensor_data (if there is a reliable way to tag/detect that provenance).
| * @details A tensor is considered read-only if it was created as a view tensor from a const pointer | ||
| * using the make_tensor() function that accepts const void* host_ptr. | ||
| * | ||
| * @param tensor OpenVINO Tensor to check | ||
| * | ||
| * @return true if the tensor is read-only, false otherwise |
There was a problem hiding this comment.
The documented definition ('view tensor from a const pointer') doesn’t match the current implementation (which checks only whether the impl is ViewTensor). Either tighten the implementation to reflect this definition, or update the documentation to describe the actual condition being tested so callers don’t rely on incorrect semantics.
| * @details A tensor is considered read-only if it was created as a view tensor from a const pointer | |
| * using the make_tensor() function that accepts const void* host_ptr. | |
| * | |
| * @param tensor OpenVINO Tensor to check | |
| * | |
| * @return true if the tensor is read-only, false otherwise | |
| * @details A tensor is considered read-only when its underlying implementation is a view | |
| * tensor that represents a non-mutable view on existing memory. This typically includes | |
| * tensors created by the make_tensor() overload that accepts const void* host_ptr, as | |
| * well as other APIs that may return ViewTensor-based read-only views. | |
| * | |
| * @note The exact condition checked by this function is whether the internal ITensor | |
| * implementation is of type ViewTensor; callers must not rely on more specific semantics | |
| * such as how the tensor view was originally constructed. | |
| * | |
| * @param tensor OpenVINO Tensor to check | |
| * | |
| * @return true if the tensor is read-only according to the above condition, false otherwise |
Details:
Tickets: