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.

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

Npu allocator #437

Closed
wants to merge 39 commits into from
Closed

Npu allocator #437

wants to merge 39 commits into from

Conversation

saurabhkale17
Copy link

Description

draft PR for remote tensor feature implementation in OVEP

javier-intel and others added 20 commits August 21, 2024 15:16
Crashing on tensor destruction. Might have UMD exceptions. Needs further
debug. Unknown if values are correct.
fix: Fixed model_proto serialized dump in Debug
Upgrade Openvino version to 2024.3.0
onnxruntime/core/providers/openvino/ov_allocator.cc Dismissed Show dismissed Hide dismissed
onnxruntime/core/providers/openvino/ov_allocator.h Dismissed Show dismissed Hide dismissed
@saurabhkale17 saurabhkale17 requested a review from vthaniel August 29, 2024 08:50
@@ -50,6 +50,15 @@ constexpr const char* HIP = "Hip";
constexpr const char* HIP_PINNED = "HipPinned";
constexpr const char* OpenVINO_CPU = "OpenVINO_CPU";
constexpr const char* OpenVINO_GPU = "OpenVINO_GPU";
constexpr const char* OpenVINO_NPU = "OpenVINO_RT_NPU";

Choose a reason for hiding this comment

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

OpenVINO_NPU is a redefinition of OpenVINO_RT_NPU.
Remove if its not referenced in the code.

Copy link
Author

Choose a reason for hiding this comment

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

not referenced so removed it in the new pr

size_t batch_size = 1;
Ort::UnownedValue output_tensor =
GetOutputTensor(context, batch_size, infer_request, std::move(output_name), subgraph_context_.output_names);
auto mem_info = output_tensor.GetTensorMemoryInfo();
if (mem_info.GetAllocatorName() == OpenVINO_GPU) {

Choose a reason for hiding this comment

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

Check if this has effect on OpenVINO_GPU IOBuffer

if (mem_info.GetAllocatorName() == OpenVINO_GPU) {
return;
auto allocator_name = output_tensor.GetTensorMemoryInfo().GetAllocatorName();
ov_tensor_data_t ov_tensor_data;

Choose a reason for hiding this comment

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

check if the declaration in startasyncinference is redundant

Copy link
Author

Choose a reason for hiding this comment

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

we will require the ov_tensor_data for creating the input/output tensor before the inference in startasyncinference

@@ -854,6 +862,25 @@ select from 'TF8', 'TF16', 'UINT8', 'FLOAT', 'ITENSOR'. \n)");
input_names_str_[i] = m.GetInputName(i);
input_names_[i] = input_names_str_[i].c_str();
}

Choose a reason for hiding this comment

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

Encapsulate the output tensor creation only if use_device_mem is set

Copy link
Author

Choose a reason for hiding this comment

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

made the relevant changes only if the use_device_mem is true in the new PR

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.

7 participants