Skip to content

Commit

Permalink
Improved telemetry
Browse files Browse the repository at this point in the history 
Fixed issue in case unsupported node is in body-graph, but
convert isn't reporting about conversion failure
  • Loading branch information
@gkrivor
gkrivor committed Mar 11, 2024
1 parent 6fc377a commit 4f2728f
Show file tree
Hide file tree
Showing 6 changed files with 98 additions and 33 deletions.
24 changes: 21 additions & 3 deletions src/frontends/onnx/frontend/src/core/graph.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,11 @@ void Graph::convert_to_ov_nodes() {
// Process ONNX graph nodes, convert to OV nodes
for (const auto& node_proto : m_model->get_graph().node()) {
if (m_extensions.telemetry) {
op_statistics[node_proto.op_type()]++;
std::string op_name =
(node_proto.has_domain() && node_proto.domain() != ""
? "***." + node_proto.op_type() + "-X"
: node_proto.op_type() + +"-" + std::to_string(m_model->get_opset_version(node_proto.domain())));
op_statistics[op_name]++;
}
const Node node{node_proto, this};
if (!m_model->is_operator_available(node.op_type(), node.domain())) {
Expand Down Expand Up @@ -275,7 +279,11 @@ void Graph::decode_to_framework_nodes() {
// Process ONNX graph nodes, convert to OV nodes
for (const auto& node_proto : m_model->get_graph().node()) {
if (m_extensions.telemetry) {
op_statistics[node_proto.op_type()]++;
std::string op_name =
(node_proto.has_domain() && node_proto.domain() != ""
? "***." + node_proto.op_type() + "-X"
: node_proto.op_type() + +"-" + std::to_string(m_model->get_opset_version(node_proto.domain())));
op_statistics[op_name]++;
}
const Node node{node_proto, this};
ov::OutputVector ov_nodes{make_framework_nodes(node)};
Expand Down Expand Up @@ -340,6 +348,8 @@ ov::OutputVector Graph::get_ov_outputs() {
ov::OutputVector Graph::make_ov_nodes(const Node& onnx_node) {
ov::OutputVector ov_subgraph_outputs;
std::string error_message;
const std::string onnx_prefix = "[ONNX Frontend] ";
// Failed to convert operation "
if (m_model->is_operator_available(onnx_node.op_type(), onnx_node.domain())) {
const auto ng_node_factory = m_model->get_operator(onnx_node.op_type(), onnx_node.domain());
try {
Expand All @@ -348,7 +358,7 @@ ov::OutputVector Graph::make_ov_nodes(const Node& onnx_node) {
error_message = e.what();
} catch (const std::exception& exc) {
error_message = error::detail::get_error_msg_prefix(onnx_node);
error_message += ":\n" + std::string{exc.what()};
error_message += ": " + std::string{exc.what()};
} catch (...) {
error_message = error::detail::get_error_msg_prefix(onnx_node);
// Since we do not know anything about current exception data type we can only
Expand All @@ -357,6 +367,14 @@ ov::OutputVector Graph::make_ov_nodes(const Node& onnx_node) {
}
}
if (ov_subgraph_outputs.empty()) { // translation not possible (not supported op or exception during processing)
if (m_extensions.telemetry && !error_message.empty()) {
std::string onnx_domain = onnx_node.domain();
int64_t opset_version = m_model->get_opset_version(onnx_domain);
error_message = onnx_prefix + "Conversion failed for " +
(onnx_domain != "" ? "***." + onnx_node.op_type() + "-X"
: onnx_node.op_type() + "-" + std::to_string(opset_version)) +
"\n" + error_message;
}
const auto not_supported_node =
std::make_shared<ov::frontend::onnx::NotSupportedONNXNode>(onnx_node.get_ov_inputs(),
onnx_node.get_outputs_size(),
Expand Down
11 changes: 11 additions & 0 deletions src/frontends/onnx/frontend/src/core/model.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,17 @@ class Model {
///
void enable_opset_domain(const std::string& domain, const OperatorsBridge& ops_bridge);

/// \brief Returns opset version for requested domain. If opset version isn't found
/// method returns -1
/// \param[in] domain The domain name.
std::int64_t get_opset_version(const std::string& domain) {
try {
return ov::frontend::onnx::get_opset_version(*this->m_model_proto, domain);
} catch (ov::Exception) {
return -1;
}
}

private:
const std::shared_ptr<ModelProto> m_model_proto;
ModelOpSet m_opset;
Expand Down
2 changes: 1 addition & 1 deletion src/frontends/onnx/frontend/src/exceptions.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ namespace onnx_error {
namespace detail {
std::string get_error_msg_prefix(const ov::frontend::onnx::Node& node) {
std::stringstream ss;
ss << "[ONNX Frontend] Node '" << node << "'";
ss << "While validating ONNX node '" << node << "'";
return ss.str();
}
} // namespace detail
Expand Down
4 changes: 2 additions & 2 deletions src/frontends/onnx/frontend/src/onnx_framework_node.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,11 +97,11 @@ class ONNXSubgraphFrameworkNode : public ONNXFrameworkNode {
// Be careful with using protobuf references (also ov::frontend::onnx::Node) inside NotSupportedONNXNode
// which are inserted into ov::Model due to different lifetime and problematic sharing between dynamic libs.
class NotSupportedONNXNode : public ov::op::util::FrameworkNode {
static constexpr const char* failed_conversion_key = "onnx::NotSupportedONNXNode::failed_conversion_key";

public:
OPENVINO_OP("NotSupportedONNXNode", "util", ov::op::util::FrameworkNode);

static constexpr const char* failed_conversion_key = "onnx::NotSupportedONNXNode::failed_conversion_key";

NotSupportedONNXNode(const ov::OutputVector& inputs,
const size_t output_size,
const std::string& domain,
Expand Down
82 changes: 56 additions & 26 deletions src/frontends/onnx/frontend/src/utils/common.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@
#include "openvino/op/broadcast.hpp"
#include "openvino/op/concat.hpp"
#include "openvino/op/divide.hpp"
#include "openvino/op/if.hpp"
#include "openvino/op/logical_and.hpp"
#include "openvino/op/loop.hpp"
#include "openvino/op/multiply.hpp"
#include "openvino/op/range.hpp"
#include "openvino/op/reshape.hpp"
Expand Down Expand Up @@ -162,15 +164,25 @@ bool is_optimized_out(const ov::Output<ov::Node>& node_output) {

bool collect_translation_exceptions(const std::shared_ptr<ov::Model>& partially_converted,
const std::shared_ptr<ov::frontend::TelemetryExtension>& telemetry,
std::ostream* output_stream) {
std::string additional_error_message = "";
std::ostream* output_stream,
std::shared_ptr<std::set<std::string>> unsupported_operations,
std::shared_ptr<std::set<std::string>> failures) {
if (partially_converted == nullptr) {
return false;
}

const std::string sep = ", ";
std::set<std::string> unsupported_operations;
std::set<std::string> failures;
bool first_call = !unsupported_operations || !failures;
if (!unsupported_operations) {
unsupported_operations = std::make_shared<std::set<std::string>>();
}
if (!failures) {
failures = std::make_shared<std::set<std::string>>();
}

auto print_unsupported = [&](const std::shared_ptr<ov::op::util::FrameworkNode> fw_node) {
if (output_stream) {
if (unsupported_operations.size() == 0) {
if (unsupported_operations->size() == 0) {
*output_stream << "OpenVINO does not support the following ONNX operations: ";
} else {
*output_stream << sep;
Expand All @@ -186,56 +198,74 @@ bool collect_translation_exceptions(const std::shared_ptr<ov::Model>& partially_
if (const auto& fw_node = std::dynamic_pointer_cast<ov::frontend::onnx::ONNXFrameworkNode>(node)) {
const auto& attrs = fw_node->get_attrs();
auto node_name = attrs.get_opset_name() + "." + attrs.get_type_name();
if (unsupported_operations.count(node_name) > 0) {
if (unsupported_operations->count(node_name) > 0) {
continue;
}

print_unsupported(fw_node);
unsupported_operations.insert(node_name);
unsupported_operations->insert(node_name);
} else if (const auto& fw_node = std::dynamic_pointer_cast<ov::frontend::onnx::NotSupportedONNXNode>(node)) {
const auto& attrs = fw_node->get_attrs();

if (fw_node->additional_error_message().empty()) {
auto node_name = attrs.get_opset_name() + "." + attrs.get_type_name();
if (unsupported_operations.count(node_name) > 0) {
if (unsupported_operations->count(node_name) > 0) {
continue;
}
print_unsupported(fw_node);
unsupported_operations.insert(node_name);
unsupported_operations->insert(node_name);
} else {
additional_error_message += fw_node->additional_error_message();
const auto node_fail = fw_node->additional_error_message();
if (failures->count(node_fail) > 0) {
continue;
}
failures->insert(node_fail);
}

const auto node_fail = attrs.find(ov::frontend::onnx::NotSupportedONNXNode::failed_conversion_key);
if (node_fail == attrs.end() || failures.count(node_fail->second) > 0) {
continue;
}
failures.insert(node_fail->second);
} else if (const auto& if_node = std::dynamic_pointer_cast<ov::op::v8::If>(node)) {
collect_translation_exceptions(if_node->get_then_body(),
telemetry,
output_stream,
unsupported_operations,
failures);
collect_translation_exceptions(if_node->get_else_body(),
telemetry,
output_stream,
unsupported_operations,
failures);
} else if (const auto& loop_node = std::dynamic_pointer_cast<ov::op::v5::Loop>(node)) {
collect_translation_exceptions(loop_node->get_function(),
telemetry,
output_stream,
unsupported_operations,
failures);
}
}

for (auto item : unsupported_operations) {
std::cerr << "Unsupported: " << item << std::endl;
}

for (auto item : failures) {
std::cerr << "Failure: " << item << std::endl;
if (!first_call) {
return unsupported_operations->size() != 0 || failures->size() != 0;
}

if (telemetry) {
for (const auto& op : unsupported_operations) {
for (const auto& op : *unsupported_operations) {
telemetry->send_event("error_cause", "onnx_" + op);
}
for (const auto& str : failures) {
for (const auto& str : *failures) {
telemetry->send_event("error_info", ov::util::filter_lines_by_prefix(str, "[ONNX Frontend] "));
}
}

if (output_stream && !additional_error_message.empty()) {
*output_stream << "Errors during ONNX translation: \n" << additional_error_message;
if (output_stream && failures->size() > 0) {
if (unsupported_operations->size() > 0) {
*output_stream << std::endl;
}
*output_stream << "Errors during ONNX translation: \n";
for (const auto& failure_message : *failures) {
*output_stream << failure_message << std::endl;
}
}

return unsupported_operations.size() != 0 || failures.size() != 0;
return unsupported_operations->size() != 0 || failures->size() != 0;
}

} // namespace common
Expand Down
8 changes: 7 additions & 1 deletion src/frontends/onnx/frontend/src/utils/common.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,10 +158,16 @@ bool is_optimized_out(const ov::Output<ov::Node>& node_output);
/// \param partially_converted ov::Model which has been partially converted
/// \param telemetry Pointer on a TelemetryExtension if telemetry is enabled
/// \param output_stream Pointer on a stream for printint error messages
/// \param unsupported_operations Set for collecting list of unsupported operations, should be nullptr for
/// first call (will be created internally)
/// \param failures Set for collecting list of failed conversions, should be nullptr for
/// first call (will be created internally)
/// \return Returns true in case any issues has been found
bool collect_translation_exceptions(const std::shared_ptr<ov::Model>& partially_converted,
const std::shared_ptr<ov::frontend::TelemetryExtension>& telemetry = nullptr,
std::ostream* output_stream = nullptr);
std::ostream* output_stream = nullptr,
std::shared_ptr<std::set<std::string>> unsupported_operations = nullptr,
std::shared_ptr<std::set<std::string>> failures = nullptr);
} // namespace common
} // namespace onnx
} // namespace frontend
Expand Down

0 comments on commit 4f2728f

Please sign in to comment.