Cleanup a change to ExecutionFrame a little (#7576)

* Reduce the binary size growth from this change. Minimal build grew by 7KB from this checkin.

Firstly simplify the checking logic a little. Same checks are still done - just without using an extra layer of helpers.

The issue being addressed by the original change only applies if you have a graph output where the shape wasn't able to be inferred. e.g. Reshape node with dynamic input causes downstream shapes to be unknown. If that is not the case, MergeShapeInfo in graph.cc would have resolved any differences between a specified output shape and the inferred output shape during Graph::Resolve.

The issue does not apply to the execution frame used by the optimizer as  the only time it would create a graph output is if it could constant fold all the way through, so MergeShapeInfo would have handled any difference in that case as well.

Due to these considerations, wiring a logger in at the IExecutionFrame level isn't necessary if VerifyOutputSizes optionally overridden by an implementation that cares.

* Address PR comments
This commit is contained in:
Scott McKay 2021-05-06 19:29:34 +10:00 committed by GitHub
parent be2a3046fe
commit 097bab8d1e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 44 additions and 73 deletions

View file

@ -52,7 +52,6 @@ Status IExecutionFrame::SetOutputMLValue(int index, const OrtValue& ort_value) {
}
void IExecutionFrame::UpdateFeeds(const std::vector<int>& feed_mlvalue_idxs, const std::vector<OrtValue>& feeds) {
ORT_ENFORCE(feed_mlvalue_idxs.size() == feeds.size());
for (size_t idx = 0, end = feed_mlvalue_idxs.size(); idx < end; ++idx) {
@ -66,10 +65,8 @@ void IExecutionFrame::UpdateFeeds(const std::vector<int>& feed_mlvalue_idxs, con
}
void IExecutionFrame::UpdateFetches(const std::vector<int>& fetch_mlvalue_idxs, const std::vector<OrtValue>& fetches, const std::unordered_map<int, OrtValue>& initializers) {
ORT_ENFORCE(fetch_mlvalue_idxs.size() == fetches.size());
if (!fetches.empty()) {
fetch_mlvalue_idxs_ = fetch_mlvalue_idxs;
@ -138,7 +135,8 @@ OrtValue* IExecutionFrame::GetMutableNodeInputOrOutputMLValue(int index) {
// This method is not thread safe!
// Return S_OK and nullptr if index map to an value that is an unused optional input/output
Status IExecutionFrame::GetOrCreateNodeOutputMLValue(const int output_index, int output_arg_index, const TensorShape* shape, OrtValue*& p_ort_value,
Status IExecutionFrame::GetOrCreateNodeOutputMLValue(const int output_index, int output_arg_index,
const TensorShape* shape, OrtValue*& p_ort_value,
const Node& node, size_t nnz) {
auto status = Status::OK();
int ort_value_idx = GetNodeIdxToMLValueIdx(output_arg_index);
@ -158,8 +156,9 @@ Status IExecutionFrame::GetOrCreateNodeOutputMLValue(const int output_index, int
" Requested shape:", shape ? shape->ToString() : "null");
}
} else {
if (this->IsOutput(ort_value_idx)) {
VerifyOutputSizes(output_index, node, shape);
// shape is nullptr for traditional ML output values
if (shape != nullptr && IsOutput(ort_value_idx)) {
VerifyOutputSizes(output_index, node, *shape);
}
status = CreateNodeOutputMLValueImpl(*p_ort_value, ort_value_idx, shape, nnz);
}
@ -168,51 +167,6 @@ Status IExecutionFrame::GetOrCreateNodeOutputMLValue(const int output_index, int
return status;
}
void IExecutionFrame::VerifyOutputSizes(int output_index, const Node& node,
const TensorShape* output_shape) {
ProtoHelperNodeContext protoContext(node);
OpNodeProtoHelper<ProtoHelperNodeContext> info(&protoContext);
const onnx::TypeProto* outputproto = info.GetOutputType(output_index);
if (outputproto == nullptr) {
return;
}
if (outputproto->value_case() != onnx::TypeProto::kTensorType) {
return;
}
const auto& tensortype = outputproto->tensor_type();
if (tensortype.has_shape()) {
const auto& shape = tensortype.shape();
int actual_num_dims = static_cast<int>(output_shape->NumDimensions());
int expected_num_dims = shape.dim_size();
if (actual_num_dims != expected_num_dims) {
if (GetLogger() != nullptr) {
LOGS(*GetLogger(), WARNING) << "Number of dimension(" << actual_num_dims << ") of output shape didn't match "
<< "with model's expected number("<< expected_num_dims <<") of dimension of output shape";
}
return;
}
for (uint32_t output_dim = 0, end = actual_num_dims; output_dim < end; ++output_dim) {
const auto dim = shape.dim(output_dim);
if (dim.has_dim_value()) {
int64_t expected_size = dim.dim_value();
int64_t actual_size = (*output_shape)[output_dim];
if (expected_size != actual_size) {
if (GetLogger() != NULL) {
LOGS(*GetLogger(), WARNING) << "Actual dimension(" << actual_size << ") didn't match with expected dimension ("
<< expected_size << ") for outputProtoIndex: " << output_index;
}
return;
}
}
}
}
}
bool IExecutionFrame::TryGetInferredShape(int /*index*/, TensorShape& /*shape*/) const {
// By default, there is not information about inferred shape, so this default
// implementation always returns false. The derived class of IExecutionFrame
@ -735,10 +689,6 @@ Status ExecutionFrame::AllocateAsPerAllocationPlan(OrtValue& ort_value, int ort_
}
}
const logging::Logger* ExecutionFrame::GetLogger() {
return &(session_state_.Logger());
}
AllocatorPtr ExecutionFrame::GetAllocatorImpl(const OrtMemoryInfo& info) const {
return session_state_.GetAllocator(info);
}
@ -750,6 +700,33 @@ Status ExecutionFrame::CreateNodeOutputMLValueImpl(OrtValue& ort_value, int ort_
return AllocateAsPerAllocationPlan(ort_value, ort_value_idx, shape, nnz);
}
void ExecutionFrame::VerifyOutputSizes(int output_index, const Node& node, const TensorShape& output_shape) {
const NodeArg* output_def = node.OutputDefs()[output_index];
const auto* expected_shape = output_def->Shape();
if (expected_shape == nullptr) {
// model didn't specify shape and shape inferencing wasn't able to calculate it so nothing to compare against
return;
}
const size_t expected_rank = expected_shape->dim_size();
bool compatible = expected_rank == output_shape.NumDimensions();
if (compatible) {
for (size_t i = 0; i < expected_rank; ++i) {
const auto& expected_dim = expected_shape->dim().Get(static_cast<int>(i));
if (expected_dim.has_dim_value() && expected_dim.dim_value() != output_shape[i]) {
compatible = false;
break;
}
}
}
if (!compatible) {
LOGS(session_state_.Logger(), WARNING) << "Expected shape from model of " << *expected_shape
<< " does not match actual shape of " << output_shape
<< " for output " << output_def->Name();
}
}
Status ExecutionFrame::ReleaseMLValueImpl(int ort_value_idx) {
ORT_RETURN_IF_ERROR(IExecutionFrame::ReleaseMLValueImpl(ort_value_idx));
TraceFree(ort_value_idx);

View file

@ -53,7 +53,8 @@ class IExecutionFrame {
// Override the index-th output with ort_value
Status SetOutputMLValue(int index, const OrtValue& ort_value);
void UpdateFeeds(const std::vector<int>& feed_mlvalue_idxs, const std::vector<OrtValue>& feeds);
void UpdateFetches(const std::vector<int>& fetch_mlvalue_idxs, const std::vector<OrtValue>& fetches, const std::unordered_map<int, OrtValue>& initializers);
void UpdateFetches(const std::vector<int>& fetch_mlvalue_idxs, const std::vector<OrtValue>& fetches,
const std::unordered_map<int, OrtValue>& initializers);
Status GetOutputs(const std::vector<int>& fetch_mlvalue_idxs, std::vector<OrtValue>& fetches);
#endif
@ -61,11 +62,11 @@ class IExecutionFrame {
// This method is not thread safe!
// Return S_OK and nullptr if index map to an value that is an unused optional input/output
// Shape is required for tensors but not traditional ML values.
Status GetOrCreateNodeOutputMLValue(const int index, int output_arg_index, const TensorShape* shape,
Status GetOrCreateNodeOutputMLValue(const int index, int output_arg_index, const TensorShape* shape,
OrtValue*& p_ort_value, const Node& node, size_t nnz = 0);
// This function try retrieve the inferred shapes for the given NodeArg index.
// If the retrival is sucessful, this function returns true and false otherwise.
// If the retrieval is successful, this function returns true and false otherwise.
virtual bool TryGetInferredShape(int index, TensorShape& shape) const;
/**
@ -97,10 +98,9 @@ class IExecutionFrame {
return all_values_[ort_value_index];
}
void VerifyOutputSizes(int output_index, const onnxruntime::Node& node,
const onnxruntime::TensorShape* output_shape);
virtual const logging::Logger* GetLogger() = 0;
// optional function that can check if the requested output_shape matched what was specified/inferred
// for the node.
virtual void VerifyOutputSizes(int /*output_index*/, const Node& /*node*/, const TensorShape& /*output_shape*/) {}
virtual AllocatorPtr GetAllocatorImpl(const OrtMemoryInfo& info) const = 0;
@ -180,10 +180,10 @@ class ExecutionFrame final : public IExecutionFrame {
private:
ORT_DISALLOW_COPY_ASSIGNMENT_AND_MOVE(ExecutionFrame);
const logging::Logger* GetLogger() override;
AllocatorPtr GetAllocatorImpl(const OrtMemoryInfo& info) const override;
Status ReleaseMLValueImpl(int ort_value_idx) override;
Status CreateNodeOutputMLValueImpl(OrtValue& ort_value, int ort_value_idx, const TensorShape* shape, size_t nnz) override;
void VerifyOutputSizes(int output_index, const Node& node, const TensorShape& output_shape) override;
Status CopyTensor(const Tensor& src, Tensor& dest) const override;
common::Status AllocateAsPerAllocationPlan(OrtValue& ort_value, int ort_value_index, const TensorShape* shape,
@ -223,7 +223,7 @@ class ExecutionFrame final : public IExecutionFrame {
// Given the input shapes of the executed graph, ExecutionFrame tries inferring
// all symbolic shapes. inferred_shapes_[i] is the shape of OrtValue indexed
// by i, if the key i exists.
// inferred_shapes_ is generated togehter with mem_patterns_.
// inferred_shapes_ is generated together with mem_patterns_.
std::unordered_map<int, TensorShape> inferred_shapes_;
// Size of virtual memory allocated before any kernel execution.

View file

@ -112,7 +112,7 @@ std::unique_ptr<const OpKernel> OptimizerExecutionFrame::Info::CreateKernel(cons
// For optimizer, probably no need to pass feed_mlvalue_idxs, feeds to initialize IExecutionFrame.
// If needed, the parameters of OptimizerExecutionFrame ctor can be changed later.
OptimizerExecutionFrame::OptimizerExecutionFrame(const Info& info,
OptimizerExecutionFrame::OptimizerExecutionFrame(const Info& info,
const std::vector<int>& fetch_mlvalue_idxs,
const std::vector<OrtValue>& fetches)
: IExecutionFrame(info.GetMLValueNameIdxMap(), info.GetNodeIndexInfo(), fetch_mlvalue_idxs),
@ -120,10 +120,6 @@ OptimizerExecutionFrame::OptimizerExecutionFrame(const Info& info,
Init(std::vector<int>(), std::vector<OrtValue>(), info.GetInitializers(), fetches);
}
const logging::Logger* OptimizerExecutionFrame::GetLogger() {
return nullptr;
}
AllocatorPtr OptimizerExecutionFrame::GetAllocatorImpl(const OrtMemoryInfo& info) const {
return info_.GetAllocator(info);
}
@ -168,8 +164,8 @@ Status OptimizerExecutionFrame::CreateNodeOutputMLValueImpl(OrtValue& ort_value,
auto element_type = static_cast<const TensorTypeBase*>(ml_type)->GetElementType();
AllocatorPtr allocator_ptr = info_.GetAllocator();
std::unique_ptr<Tensor> p_tensor = std::make_unique<Tensor>(element_type,
*shape,
allocator_ptr);
*shape,
allocator_ptr);
auto ml_tensor = DataTypeImpl::GetType<Tensor>();
ort_value.Init(p_tensor.release(), ml_tensor, ml_tensor->GetDeleteFunc());

View file

@ -88,8 +88,6 @@ class OptimizerExecutionFrame final : public IExecutionFrame {
private:
ORT_DISALLOW_COPY_ASSIGNMENT_AND_MOVE(OptimizerExecutionFrame);
const logging::Logger* GetLogger() override;
AllocatorPtr GetAllocatorImpl(const OrtMemoryInfo& info) const override;
Status CreateNodeOutputMLValueImpl(OrtValue& ort_value, int ort_value_idx, const TensorShape* shape, size_t nnz) override;