From 068d31511b96fabcb3116dd125f446afaa062eb7 Mon Sep 17 00:00:00 2001 From: Ilya Lavrenov Date: Tue, 28 Sep 2021 12:50:51 +0300 Subject: [PATCH] Improved ov::Tensor behavior (#7683) * Improved ov::Tensor behavior * Fixed python test for setShape on preallocated * Fixed clang-format --- .../ie_bridges/python/tests/test_Blob.py | 9 ++- .../src/inference_engine/include/ie/ie_blob.h | 17 +----- .../inference_engine/include/ie/ie_layouts.h | 8 +-- .../inference_engine/src/ie_blob_common.cpp | 47 +++++++++++++++ .../src/inference_engine/src/ie_layouts.cpp | 15 +++++ ngraph/core/CMakeLists.txt | 1 + .../core/include/openvino/runtime/tensor.hpp | 8 +-- ngraph/core/src/runtime/ov_tensor.cpp | 15 +---- ngraph/test/ov_tensor_test.cpp | 59 ++++++++++++++++--- 9 files changed, 127 insertions(+), 52 deletions(-) diff --git a/inference-engine/ie_bridges/python/tests/test_Blob.py b/inference-engine/ie_bridges/python/tests/test_Blob.py index 14624fa3daa..5c69387d395 100644 --- a/inference-engine/ie_bridges/python/tests/test_Blob.py +++ b/inference-engine/ie_bridges/python/tests/test_Blob.py @@ -130,11 +130,14 @@ def test_set_shape(): assert blob.tensor_desc.dims == [1, 4, 128, 128] assert blob.buffer.shape == (1, 4, 128, 128) + +def test_cannot_set_shape_preallocated_memory(): + tensor_desc = TensorDesc("FP32", [1, 3, 127, 127], "NHWC") array = np.ones([1, 3, 127, 127], dtype=np.float32) blob = Blob(tensor_desc, array) - blob.set_shape([1, 4, 128, 128]) - assert blob.tensor_desc.dims == [1, 4, 128, 128] - assert blob.buffer.shape == (1, 4, 128, 128) + with pytest.raises(RuntimeError) as e: + blob.set_shape([1, 4, 128, 128]) + assert "Cannot call setShape for Blobs created on top of preallocated memory" in str(e.value) @pytest.mark.ngraph_dependent_test diff --git a/inference-engine/src/inference_engine/include/ie/ie_blob.h b/inference-engine/src/inference_engine/include/ie/ie_blob.h index aa625e6f33c..3302b850282 100644 --- a/inference-engine/src/inference_engine/include/ie/ie_blob.h +++ b/inference-engine/src/inference_engine/include/ie/ie_blob.h @@ -192,22 +192,7 @@ public: * * @param dims new shape */ - void setShape(const SizeVector& dims) { - if (properProduct(dims) > properProduct(getTensorDesc().getDims())) { - // New blob shape requires more memory than old one -- reallocate - if (!deallocate()) - IE_THROW() << "Cannot deallocate blob while an attempt to enlarge blob area in setShape."; - - // Old and new ranks should match as well as layouts - getTensorDesc().setDims(dims); - - allocate(); - // no way to detect if allocation is successful other than map/unmap that we wouldn't like to do here - } else { - // Don't shrink area when new size fit the existing area - getTensorDesc().setDims(dims); - } - } + void setShape(const SizeVector& dims); /** * @deprecated Cast to MemoryBlob and use new wlock/rwlock API instead. diff --git a/inference-engine/src/inference_engine/include/ie/ie_layouts.h b/inference-engine/src/inference_engine/include/ie/ie_layouts.h index 76aa5d62ae9..a2fde6c940b 100644 --- a/inference-engine/src/inference_engine/include/ie/ie_layouts.h +++ b/inference-engine/src/inference_engine/include/ie/ie_layouts.h @@ -84,7 +84,7 @@ public: /** * @brief Returns the vector of order * - * @return order + * @return order of dimensions */ const SizeVector& getOrder() const { return order; @@ -93,7 +93,7 @@ public: /** * @brief Returns the per-dimension offset vector * - * @return offsets + * @return offsets in elements */ const SizeVector& getOffsetPaddingToData() const { return offsetPaddingToData; @@ -102,7 +102,7 @@ public: /** * @brief Returns the offset to the current memory block * - * @return offset + * @return offset in elements */ size_t getOffsetPadding() const { return offsetPadding; @@ -111,7 +111,7 @@ public: /** * @brief Returns strides for each dimension * - * @return strides + * @return strides in elements */ const SizeVector& getStrides() const { return strides; diff --git a/inference-engine/src/inference_engine/src/ie_blob_common.cpp b/inference-engine/src/inference_engine/src/ie_blob_common.cpp index 9bfa02eb3aa..30e824338bc 100644 --- a/inference-engine/src/inference_engine/src/ie_blob_common.cpp +++ b/inference-engine/src/inference_engine/src/ie_blob_common.cpp @@ -7,8 +7,55 @@ #include #include "ie_blob.h" +#include "system_allocator.hpp" namespace InferenceEngine { + +void Blob::setShape(const SizeVector& dims) { + // we don't want to allow setShape for: + // 1. ROI cases + { + size_t denseStride = 1; + const auto& blockedDims = getTensorDesc().getBlockingDesc().getBlockDims(); + const auto& strides = getTensorDesc().getBlockingDesc().getStrides(); + + for (size_t i = 1; i <= strides.size(); i++) { + if (denseStride != strides[strides.size() - i]) { + IE_THROW() << "Blob::setShape requires dense blob"; + } + denseStride *= blockedDims[blockedDims.size() - i]; + } + } + + // 2. Blobs created on top of preallocated memory + if (std::dynamic_pointer_cast(getAllocator())) { + IE_THROW() << "Cannot call setShape for Blobs created on top of preallocated memory."; + } + + if (properProduct(dims) > properProduct(getTensorDesc().getDims())) { + // New blob shape requires more memory than old one -- reallocate + if (!deallocate()) { + IE_THROW() << "Cannot deallocate blob while an attempt to enlarge blob area in setShape."; + } + + // Old and new ranks should match as well as layouts + getTensorDesc().setDims(dims); + + allocate(); + // no way to detect if allocation is successful other than map/unmap + // that we wouldn't like to do here; but for cases when we use SystemMemoryAllocator + // we can do it + if (std::dynamic_pointer_cast(getAllocator())) { + if (buffer() == nullptr) { + IE_THROW() << "Failed to allocate memory in Blob::setShape"; + } + } + } else { + // Don't shrink area when new size fit the existing area + getTensorDesc().setDims(dims); + } +} + Blob::Ptr Blob::createROI(const ROI& roi) const { if (getTensorDesc().getLayout() == Layout::NCHW || getTensorDesc().getLayout() == Layout::NHWC) { return createROI({roi.id, 0, roi.posY, roi.posX}, diff --git a/inference-engine/src/inference_engine/src/ie_layouts.cpp b/inference-engine/src/inference_engine/src/ie_layouts.cpp index b8557a9b558..4046a56a6aa 100644 --- a/inference-engine/src/inference_engine/src/ie_layouts.cpp +++ b/inference-engine/src/inference_engine/src/ie_layouts.cpp @@ -288,6 +288,21 @@ BlockingDesc::BlockingDesc(const SizeVector& blocked_dims, if (blocked_dims.size() != dimOffsets.size()) IE_THROW() << "Offsets are not initialized for all dimensions."; this->offsetPaddingToData = dimOffsets; + + // check that strides are valid + { + size_t denseStride = 1; + + for (size_t i = 1; i <= strides.size(); i++) { + if (denseStride > strides[strides.size() - i]) { + IE_THROW() << "Stride in " << (strides.size() - i) + << "-th dimension " + "is not valid; actual " + << strides[strides.size() - i] << ", should be >= " << denseStride << std::endl; + } + denseStride = std::max(strides[strides.size() - i], denseStride) * blocked_dims[blocked_dims.size() - i]; + } + } } BlockingDesc::BlockingDesc(const SizeVector& dims, Layout layout) : offsetPadding(0) { diff --git a/ngraph/core/CMakeLists.txt b/ngraph/core/CMakeLists.txt index dcaaf45f391..62f207f860d 100644 --- a/ngraph/core/CMakeLists.txt +++ b/ngraph/core/CMakeLists.txt @@ -18,6 +18,7 @@ set(IE_SHARED_SRCS "${IE_SRC_ROOT}/system_allocator.cpp" "${IE_SRC_ROOT}/blob_factory.cpp" "${IE_SRC_ROOT}/ie_blob_common.cpp" + "${IE_SRC_ROOT}/system_allocator.cpp" "${IE_SRC_ROOT}/ie_layouts.cpp") set(MIXED_SRC ${IE_SHARED_SRCS} "${CMAKE_CURRENT_SOURCE_DIR}/src/runtime/allocator.cpp" diff --git a/ngraph/core/include/openvino/runtime/tensor.hpp b/ngraph/core/include/openvino/runtime/tensor.hpp index 4061f528672..6abfc3247e8 100644 --- a/ngraph/core/include/openvino/runtime/tensor.hpp +++ b/ngraph/core/include/openvino/runtime/tensor.hpp @@ -69,16 +69,10 @@ public: * @param type Tensor element type * @param shape Tensor shape * @param host_ptr Pointer to pre-allocated host memory - * @param size Optional size of allocated host memory in elements. If it is not set (default is `0`), the size of - * memory supposed to be not less then ov::shape_size(shape) * type.size() in bytes. * @param strides Optional strides parameters in elements. Strides are supposed to be equal to shape if they are not * set */ - Tensor(const element::Type type, - const Shape& shape, - void* host_ptr, - const size_t size = 0, - const Strides& strides = {}); + Tensor(const element::Type type, const Shape& shape, void* host_ptr, const Strides& strides = {}); /** * @brief Constructs region of interest (ROI) tensor form another tensor. diff --git a/ngraph/core/src/runtime/ov_tensor.cpp b/ngraph/core/src/runtime/ov_tensor.cpp index 73a6c60a84b..1c4a06c039e 100644 --- a/ngraph/core/src/runtime/ov_tensor.cpp +++ b/ngraph/core/src/runtime/ov_tensor.cpp @@ -38,11 +38,7 @@ Tensor::Tensor(const element::Type element_type, const Shape& shape, const Alloc _impl->allocate(); } -Tensor::Tensor(const element::Type element_type, - const Shape& shape, - void* host_ptr, - const size_t size, - const Strides& strides) { +Tensor::Tensor(const element::Type element_type, const Shape& shape, void* host_ptr, const Strides& strides) { ie::SizeVector blk_order(shape.size()); std::iota(blk_order.begin(), blk_order.end(), 0); ie::SizeVector dim_offset(shape.size(), 0); @@ -50,12 +46,6 @@ Tensor::Tensor(const element::Type element_type, if (strides.empty()) { blk_strides = ov::row_major_strides(shape); } else { - OPENVINO_ASSERT(shape.size() == strides.size(), - "shape.size() (", - shape.size(), - ") must be equal to strides.size() (", - strides.size(), - ")"); blk_strides.assign(strides.begin(), strides.end()); } @@ -64,8 +54,7 @@ Tensor::Tensor(const element::Type element_type, ie::TensorDesc{ie::details::convertPrecision(element_type), shape, ie::BlockingDesc{shape, blk_order, 0, dim_offset, blk_strides}}, - host_ptr, - size); + host_ptr); } catch (const std::exception& ex) { throw ov::Exception(ex.what()); } catch (...) { diff --git a/ngraph/test/ov_tensor_test.cpp b/ngraph/test/ov_tensor_test.cpp index ee19938ada1..9867ffb79f8 100644 --- a/ngraph/test/ov_tensor_test.cpp +++ b/ngraph/test/ov_tensor_test.cpp @@ -11,6 +11,7 @@ #include #include +#include "openvino/core/except.hpp" #include "openvino/runtime/allocator.hpp" #include "openvino/runtime/tensor.hpp" @@ -58,7 +59,7 @@ TEST_F(OVTensorTest, canCreateTensorUsingMockAllocator) { TEST_F(OVTensorTest, canAccessExternalData) { ov::Shape shape = {1, 1, 3}; float data[] = {5.f, 6.f, 7.f}; - ov::runtime::Tensor t{ov::element::f32, shape, data, 3}; + ov::runtime::Tensor t{ov::element::f32, shape, data}; { float* ptr = t.data(); ASSERT_EQ(ptr[2], 7); @@ -74,7 +75,8 @@ TEST_F(OVTensorTest, canAccessExternalData) { TEST_F(OVTensorTest, canAccessExternalDataWithStrides) { ov::Shape shape = {2, 3}; float data[] = {5.f, 6.f, 7.f, 0.f, 1.f, 42.f, 3.f, 0.f}; - ov::runtime::Tensor t{ov::element::f32, shape, data, 8, {4, 1}}; + ov::runtime::Tensor t{ov::element::f32, shape, data, {4, 1}}; + ASSERT_EQ(ov::Strides({4, 1}), t.get_strides()); { ASSERT_EQ((ov::Shape{2, 3}), t.get_shape()); float* ptr = t.data(); @@ -90,7 +92,17 @@ TEST_F(OVTensorTest, cannotCreateTensorWithExternalNullptr) { TEST_F(OVTensorTest, cannotCreateTensorWithWrongStrides) { ov::Shape shape = {2, 3}; float data[] = {5.f, 6.f, 7.f, 0.f, 1.f, 42.f, 3.f, 0.f}; - ASSERT_THROW(ov::runtime::Tensor(ov::element::f32, shape, data, 8, {4, 1, 2}), ov::Exception); + { + // strides.size() != shape.size() + EXPECT_THROW(ov::runtime::Tensor(ov::element::f32, shape, data, {6, 3, 1}), ov::Exception); + } + { + // strides values are element-wise >= ov::row_major_strides(shape) values + EXPECT_THROW(ov::runtime::Tensor(ov::element::f32, shape, data, {2, 1}), ov::Exception); + EXPECT_THROW(ov::runtime::Tensor(ov::element::f32, shape, data, {3, 0}), ov::Exception); + EXPECT_THROW(ov::runtime::Tensor(ov::element::f32, shape, data, {3, 2}), ov::Exception); + EXPECT_NO_THROW(ov::runtime::Tensor(ov::element::f32, shape, data, {6, 2})); + } } TEST_F(OVTensorTest, saveDimsAndSizeAfterMove) { @@ -115,36 +127,65 @@ TEST_F(OVTensorTest, saveDimsAndSizeAfterMove) { // SetShape TEST_F(OVTensorTest, canSetShape) { + const ov::Shape origShape({1, 2, 3}); ov::runtime::Tensor t{ov::element::f32, {1, 2, 3}}; const ov::Shape newShape({4, 5, 6}); - ASSERT_EQ(t.get_shape(), (ov::Shape{1, 2, 3})); + + const void* orig_data = t.data(); + ASSERT_EQ(t.get_shape(), origShape); ASSERT_NO_THROW(t.set_shape({4, 5, 6})); ASSERT_EQ(newShape, t.get_shape()); ASSERT_EQ(ov::row_major_strides(newShape), t.get_strides()); + ASSERT_NE(orig_data, t.data()); // check that setShape for copy changes original Tensor { ov::runtime::Tensor t2 = t; - t2.set_shape(newShape); + ASSERT_NO_THROW(t2.set_shape(newShape)); ASSERT_EQ(newShape, t.get_shape()); ASSERT_EQ(t2.get_shape(), t.get_shape()); + orig_data = t.data(); + } + + // set_shape for smaller memory - does not perform reallocation + { + t.set_shape(origShape); + ASSERT_EQ(origShape, t.get_shape()); + ASSERT_EQ(orig_data, t.data()); } } +TEST_F(OVTensorTest, cannotSetShapeOnPreallocatedMemory) { + float data[4 * 5 * 6 * 2]; + ov::runtime::Tensor t{ov::element::f32, {1, 2, 3}, data}; + const ov::Shape newShape({4, 5, 6}); + + ASSERT_THROW(t.set_shape(newShape), ov::Exception); +} + TEST_F(OVTensorTest, makeRangeRoiTensor) { - ov::runtime::Tensor t{ov::element::i8, {1, 3, 6, 5}}; // RGBp picture of size (WxH) = 5x6 + ov::runtime::Tensor t{ov::element::i32, {1, 3, 6, 5}}; // RGBp picture of size (WxH) = 5x6 ov::runtime::Tensor roi_tensor{t, {0, 0, 1, 2}, {1, 3, 5, 4}}; ov::Shape ref_shape = {1, 3, 4, 2}; - ptrdiff_t ref_offset = 7; + ptrdiff_t ref_offset_elems = 7; + ptrdiff_t ref_offset_bytes = ref_offset_elems * ov::element::i32.size(); ov::Strides ref_strides = {90, 30, 5, 1}; ASSERT_EQ(roi_tensor.get_shape(), ref_shape); - ASSERT_EQ(roi_tensor.data() - t.data(), ref_offset); - ASSERT_EQ(reinterpret_cast(roi_tensor.data()) - reinterpret_cast(t.data()), ref_offset); + ASSERT_EQ(roi_tensor.data() - t.data(), ref_offset_elems); + ASSERT_EQ(reinterpret_cast(roi_tensor.data()) - reinterpret_cast(t.data()), ref_offset_bytes); ASSERT_EQ(roi_tensor.get_strides(), t.get_strides()); ASSERT_EQ(ref_strides, roi_tensor.get_strides()); ASSERT_EQ(roi_tensor.get_element_type(), t.get_element_type()); } +TEST_F(OVTensorTest, cannotSetShapeOnRoiTensor) { + ov::runtime::Tensor t{ov::element::i32, {1, 3, 6, 5}}; // RGBp picture of size (WxH) = 5x6 + ov::runtime::Tensor roi_tensor{t, {0, 0, 1, 2}, {1, 3, 5, 4}}; + const ov::Shape newShape({4, 5, 6}); + + ASSERT_THROW(roi_tensor.set_shape(newShape), ov::Exception); +} + TEST_F(OVTensorTest, makeRangeRoiTensorInt4) { ov::runtime::Tensor t{ov::element::i4, {1, 6, 5, 3}}; // RGB picture of size (WxH) = 5x6 ov::runtime::Tensor roi_tensor{t, {0, 1, 2, 0}, {1, 5, 4, 3}};