checking the OpenVINO code
“OpenVINO – a set of tools that allows deep learning of AI to interact with the real world is now Open Source!” — this news sounded like a call to action for us. The project code has been checked, errors have been found, and the first part of the article is ready for reading. It will be interesting!
A few words about the project
On March 6, 2024, Intel released the Open Source solution OpenVINO. What is it? First and foremost, it is a tool to help professionals train AI computer vision more effectively.
And if you don’t know what computer vision is, this is the place for you.
Computer vision is when, using a more or less normal camera connected to a computer, we can recognize some of the data we need in a video. For example, we want to capture a white rabbit in the middle of the same white snowdrifts in winter. This will be extremely difficult to do with your eyes, but with the help of a camera and trained AI, any rabbit running past will not go unnoticed.
It is also worth noting that computer vision is extremely useful and helps in many areas, for example:
Sorting and classification of agricultural products;
Classification and identification of defects in production of various electronic modules and components;
Industrial automation;
Facial recognition systems;
The use of AI in colonoscopy, in case of deterioration of vision or in identifying brain diseases.
This is not a complete list of possibilities, but that’s not what the article is about. In any case, the usefulness of using artificial intelligence technologies is undeniable, and in order to make all this possible, various in-depth learning tools are being developed. One of such tools is OpenVINO.
More precisely, OpenVINO is a free set of ready-made utilities that developers can use to speed up the development of such solutions. Also, in addition to training tools, there are also tools for testing the effectiveness of AI and its own benchmark. Something like this.
And so, in a timid attempt to touch the beauty with my crooked hands, I again uncovered the PVS-Studio static analyzer in order to plunge into the project code to search for foreign entities called bugs. As I wrote earlier, it will be interesting.
However, despite the bugs, the tool performs its functions: the AI learns and works (judging by the numerous articles by the developers).
You will not encounter the situation described below in the practice of training your AI:
Developer: AI, I don't feel computer vision!
AI: Meow?!
The commit where I built the project for review: 2d8ac08.
Disclaimer
The writing and publication of this article is not intended to devalue the work of programmers involved in the development of this product. Goal: popularization of static code analyzers, which are useful even for high-quality and established projects.
Test results
The project under review turned out to be rich in typos and problems that arise after copy-paste.
Based on this, it was decided to write a separate article in order to highlight the fact that errors of this kind occur almost everywhere.
And yes, there is always a possibility that this is what the programmer intended, but this probability… is extremely small: “Immolate Improved!”
Fragment N1
ov::pass::ConvertPadToGroupConvolution::
ConvertPadToGroupConvolution()
{
....
const auto& pad_begin = pad->get_pads_begin();
const auto& pad_end = pad->get_pads_end();
if (pad_begin.empty() || pad_end.empty())
{
// pads will be empty if inputs are not constants
return false;
}
// check that Pad has non-negative values
auto pred = [](int64_t a)
{
return a < 0;
};
if (std::any_of(pad_begin.begin(), pad_begin.end(), pred) ||
std::any_of(pad_begin.begin(), pad_begin.end(), pred)) // <=
{
return false;
}
....
}
Analyzer warning:
V501 There are identical sub-expressions 'std::any_of(pad_begin.begin(), pad_begin.end(), pred)' to the left and to the right of the '||' operator. convert_pad_to_group_conv.cpp 66
As you can see, the bottom condition compares two identical expressions: the same function calls std::any_of with the same parameters. The code above was added to the example for a reason, because if you look at it, you can conclude that the condition should most likely be written as follows:
if (std::any_of(pad_begin.begin(), pad_begin.end(), pred) ||
std::any_of(pad_end.begin(), pad_end.end(), pred))
{
return false;
}
Fragment N2
ov::pass::ShuffleChannelsFusion::
ShuffleChannelsFusion(const bool reshape_constants_check)
{
....
auto reshape_before_constant = std::dynamic_pointer_cast
<ov::op::v0::Constant>(
pattern_map.at(reshape_before_const_pattern).get_node_shared_ptr());
auto reshape_before = std::dynamic_pointer_cast<ov::op::v1::Reshape>(
pattern_map.at(reshape_before_pattern).get_node_shared_ptr());
auto transpose = std::dynamic_pointer_cast<ov::op::v1::Transpose>(
pattern_map.at(transpose_pattern).get_node_shared_ptr());
auto reshape_after = std::dynamic_pointer_cast<ov::op::v1::Reshape>(
pattern_map.at(reshape_after_pattern).get_node_shared_ptr());
auto reshape_after_constant = std::dynamic_pointer_cast<ov::op::v0::Constant>(
pattern_map.at(reshape_after_const_pattern).get_node_shared_ptr());
if (!reshape_after || !transpose || !reshape_after || // <=
!reshape_before_constant || !reshape_after_constant)
{
return false;
}
....
}
Analyzer warning:
V501 There are identical sub-expressions '!reshape_after' to the left and to the right of the '||' operator. shuffle_channels_fusion.cpp 115
Again there is an error in the condition. The same expression is checked twice, namely that the pointer reshape_after non-zero. If we look at the code above again, we will notice the initialization reshape_before. Most likely, the condition should be rewritten as follows:
if (!reshape_after || !transpose || !reshape_before ||
!reshape_before_constant || !reshape_after_constant)
{
return false;
}
Fragment N3
....
using PatternValueMaps = std::vector<PatternValueMap>;
....
PatternValueMaps m_pattern_value_maps;
....
MatcherState::~MatcherState()
{
if (m_restore)
{
if (!m_matcher->m_matched_list.empty())
{
m_matcher->m_matched_list.erase(m_matcher->m_matched_list.begin() +
m_watermark,
m_matcher->m_matched_list.end());
}
if (!m_pattern_value_maps.empty())
{
m_matcher->m_pattern_value_maps.erase(m_pattern_value_maps.begin() + // <=
m_capture_size,
m_pattern_value_maps.end());
}
m_matcher->m_pattern_map = m_pattern_value_map;
}
}
Analyzer warning:
V539 [CERT-CTR53-CPP] Consider inspecting iterators which are being passed as arguments to function 'erase'. matcher.cpp 48
This time the error is complex. Let's look at everything in order.
First, it is checked that the vector m_pattern_value_maps not empty.
Then we notice that in then-branch of the second nested if For some reason, we are working with a different vector:* m_matcher->m_pattern_value_maps*. Further more.
To a member function std::vector
Judging by to the designer And destructorClass MatcherState designed to roll back changes object type Matcher. Perhaps earlier code RAII-the wrapper saved the state of the type object Matcher V fields MatcherState::m_pattern_value_map And MatcherState::m_pattern_value_maps and then returned it in the destructor.
However, then the code was rewritten, adding fields MatcherState::m_watermark And MatcherState::m_capture_size. They are responsible for removing elements that have been appended to the end containers Matcher::m_matched_list And Matcher::m_pattern_value_maps.
Based on the previous considerations, the code needs to be corrected as follows:
if (!m_matcher->m_pattern_value_maps.empty())
{
m_matcher->m_pattern_value_maps.erase(
m_matcher->m_pattern_value_maps.begin() +
m_capture_size,
m_matcher->m_pattern_value_maps.end() );
}
I also want to note that the field MatcherState::m_pattern_value_maps is no longer used and may need to be removed.
Fragment N4
template <x64::cpu_isa_t isa>
void jit_power_dynamic_emitter::
emit_isa(const std::vector<size_t> &in_vec_idxs,
const std::vector<size_t> &out_vec_idxs) const
{
....
if (isa == x64::avx512_core || isa == x64::avx512_core) // <=
{
h->sub(h->rsp, n_k_regs_to_save * k_mask_size);
for (size_t i = 0; i < n_k_regs_to_save; ++i)
{
if (x64::mayiuse(x64::avx512_core))
h->kmovq(h->ptr[h->rsp + i * k_mask_size], Opmask(i));
else
h->kmovw(h->ptr[h->rsp + i * k_mask_size], Opmask(i));
}
}
....
}
Analyzer warning:
V501 There are identical sub-expressions 'isa == x64::avx512_core' to the left and to the right of the '||' operator. jit_eltwise_emitters.cpp 705
And again there is an error in the condition: two absolutely identical expressions are checked. Most likely, in the second expression isa must be equal to another value.
It is noteworthy that this error occurs in the code in three additional places:
V501 There are identical sub-expressions 'isa == x64::avx512_core' to the left and to the right of the '||' operator. jit_eltwise_emitters.cpp 754
V501 There are identical sub-expressions 'isa == x64::avx512_core' to the left and to the right of the '||' operator. jit_eltwise_emitters.cpp 1609
V501 There are identical sub-expressions 'isa == x64::avx512_core' to the left and to the right of the '||' operator. jit_eltwise_emitters.cpp 1658
Fragment N5
void GridSampleKernel<isa>::reflectionPadding(const Vmm& vCoordDst,
const Vmm& vCoordOrigin,
const coord dim )
{
....
if (dim == coord::w)
{
....
} else if (coord::h) // <=
{
....
} else {....}
....
}
Analyzer warning:
V768 The enumeration constant 'h' is used as a variable of a Boolean-type. grid_sample.cpp 925
It is very strange that the second condition checks a constant coord::h. Of course, it has the value 1, and this code will always return truewhich is clearly an error.
In this case, the code in the body of the last branch else will never be fulfilled. What is this: a cunning idea of the developer or, perhaps, an artificial limitation for the executable code? It looks more like a bug, and the condition should contain the expression dim == coord::h.
And a few more similar warnings:
V768 The enumeration constant 'h' is used as a variable of a Boolean-type. grid_sample.cpp 959
V768 The enumeration constant 'h' is used as a variable of a Boolean-type. grid_sample.cpp 990
The error is also a consequence of the fact that the programmer copied the field coord::h into the condition, but forgot to compare its value with the parameter value dim.
Fragment N6
OutputVector translate_im2col(const NodeContext& context)
{
num_inputs_check(context, 5, 5);
auto input = context.get_input(0);
auto kernel_size = context.const_input<std::vector<int64_t>>(1);
PYTORCH_OP_CONVERSION_CHECK(kernel_size.size() == 2,
"kernel size should contains 2 elements");
auto dilation = context.const_input<std::vector<int64_t>>(2);
PYTORCH_OP_CONVERSION_CHECK(kernel_size.size() == 2, // <=
"dilation should contains 2 elements");
auto padding = context.const_input<std::vector<int64_t>>(3);
PYTORCH_OP_CONVERSION_CHECK(kernel_size.size() == 2, // <=
"padding should contains 2 elements");
auto stride = context.const_input<std::vector<int64_t>>(4);
PYTORCH_OP_CONVERSION_CHECK(kernel_size.size() == 2, // <=
"stride should contains 2 elements");
....
}
Analyzer warning:
V547 Expression 'kernel_size.size() == 2' is always true. im2col.cpp 65
And a few more warnings to understand the full picture:
V547 Expression 'kernel_size.size() == 2' is always true. im2col.cpp 67
V547 Expression 'kernel_size.size() == 2' is always true. im2col.cpp 69
At first glance, it is not clear what the analyzer is trying to tell us with all these warnings, but let's think about it.
The first thing that catches your eye: verification kernel_size.size() == 2 happens as many as four times. Moreover, after the first check, the vector kernel_size doesn't change anywhere. The analyzer hints at this by saying that the following three checks are always true.
How exactly did the analyzer understand this?
In a macro PYTORCH_OP_CONVERSION_CHECK There is function create, which throws an exception if the expression passed in the macro is false. Therefore, in order for all code to be reachable after the first check (and by default we consider it so), it is necessary that the expression kernel_size.size() was equal to 2.
The next thing we might think about is: why do we need to check the value at all? kernel_size.size(), if vector kernel_size does not change, and its size will always be equal to 2? Everything we have considered earlier was only a consequence of the mistake made, and not the cause. The cause is simple and is as follows.
Here the object was created and initialized kernel_sizeand then into the macro PYTORCH_OP_CONVERSION_CHECK the expression is passed and checked inside kernel_size.size() == 2. Next another object is created dilationhowever in the macro PYTORCH_OP_CONVERSION_CHECK the expression is also conveyed kernel_size.size() == 2although, if you think about it, logically, the expression should be passed and checked dilation.size() == 2.
So, if you pay attention to the second argument passed to this macro – a string, then, again, it becomes obvious that the function size must be called on an object dilation. And the same for the other two objects, well, you understand… the programmer copied and pasted lines of code and forgot to change the names for the objects, the size checks for which were passed as a parameter to the macro.
Fragment N7
void BinaryConvolution::createPrimitive()
{
....
bool args_ok = jcp.l_pad <= jcp.ur_w && // <=
(r_pad_no_tail <= jcp.ur_w) && (jcp.l_pad <= jcp.ur_w) && // <=
IMPLICATION(jcp.kw > 7, (jcp.t_pad == 0 && jcp.l_pad == 0) ||
(jcp.stride_w == 1 && jcp.stride_h == 1));
....
}
Analyzer warning:
V501 There are identical sub-expressions 'jcp.l_pad <= jcp.ur_w' to the left and to the right of the '&&' operator. bin_conv.cpp 1088
As we can observe, the same expression is checked. Nothing bad will happen, but one extra expression needs to be removed.
Fragment N8
void FakeQuantize::getSupportedDescriptors()
{
....
if (getInputShapeAtPort(0).getRank() !=
getInputShapeAtPort(0).getRank()) // <=
{
OPENVINO_THROW(errorPrefix,
"has different ranks for input and output tensors");
}
....
}
Analyzer warning:
V501 There are identical sub-expressions 'getInputShapeAtPort(0).getRank()' to the left and to the right of the '!=' operator. fake_quantize.cpp 1301
The condition tests the same subexpression for inequality. Most likely, there is a copy-paste problem involved here. And since they write to us in plain text: “has different ranks for input and output tensors“, – and if we take into account the fact that there is a similar function, but for output values:
const Shape& getOutputShapeAtPort(size_t port) const
{
if (outputShapes.size() <= port)
{
OPENVINO_THROW("Incorrect output port number for node ", getName());
}
return outputShapes[port];
}
Most likely, the condition needs to be corrected as follows:
if (getInputShapeAtPort(0).getRank() != getOutputShapeAtPort(0).getRank())
{
OPENVINO_THROW(errorPrefix,
"has different ranks for input and output tensors");
}
Fragment N9
void set_state(const ov::SoPtr<ov::ITensor>& state) override
{
OPENVINO_ASSERT(state->get_shape() ==
m_state->get_shape(),
"Wrong tensor shape.");
OPENVINO_ASSERT(state->get_element_type() ==
state->get_element_type(), // <=
"Wrong tensor type." );
OPENVINO_ASSERT(state->get_byte_size() ==
state->get_byte_size(), // <=
"Blob size of tensors are not equal.");
std::memcpy(m_state->data(), state->data(), state->get_byte_size());
}
Analyzer warnings:
V501 There are identical sub-expressions 'state->get_element_type()' to the left and to the right of the '==' operator. variable_state.hpp 23
V501 There are identical sub-expressions 'state->get_byte_size()' to the left and to the right of the '==' operator. variable_state.hpp 24
The first line compares expressions state->get_shape() And m_state->get_shape(). However, in the following lines, due to copy-paste, the results of calling member functions are compared get_element_type And get_byte_size the same object state. Most likely, this happened because the names m_state And state are similar to each other, and the programmer did not pay attention to this.
Let's fix the code:
....
OPENVINO_ASSERT(state->get_element_type() ==
m_state->get_element_type(),
"Wrong tensor type." );
OPENVINO_ASSERT(state->get_byte_size() ==
m_state->get_byte_size(),
"Blob size of tensors are not equal.");
....
Fragment N10
void SubgraphExtractor::add_new_inputs(const std::vector<
InputEdge>& new_inputs,
const bool merge_inputs )
{
....
auto it = std::find_if(new_inputs.begin(), new_inputs.begin(),
[&](const InputEdge& input_edge)
{
return get_input_tensor_name(m_onnx_graph,
input_edge) == input.first;
} );
....
}
Analyzer warning:
V539 [CERT-CTR53-CPP] Consider inspecting iterators which are being passed as arguments to function 'find_if'. subgraph_extraction.cpp 300
If we pay attention to the function call std::find_ifthen note that the second argument must be a call new_inputs.end(). In its current state the code will always return new_inputs.begin().
Let's fix the code:
....
auto it = std::find_if(new_inputs.begin(), new_inputs.end(),
[&](const InputEdge& input_edge)
{
return get_input_tensor_name(
m_onnx_graph, input_edge) == input.first;
});
....
Fragment N11
std::vector<PortConfig> inConfs;
....
MemoryDescPtr Node::getBaseMemDescAtInputPort(size_t portNum) const
{
if (auto primDesc = getSelectedPrimitiveDescriptor())
{
const auto& inConfs = primDesc->getConfig().inConfs;
if (inConfs.size() < portNum) // N1
{
OPENVINO_THROW("Can't get input memory desc at port: ",
portNum, ", incorrect port number" );
}
return inConfs[portNum].getMemDesc(); // N2
}
OPENVINO_THROW("Can't get input memory desc,
primitive descriptor is not selected");
}
Analyzer warning:
V557 [CERT-ARR30-C] Array overrun is possible. The 'portNum' index is pointing beyond array bound. node.cpp 402
At first glance, there is nothing unusual in this piece of code, and it seems that the analyzer is swearing in vain. But no. To make it easier to explain, I have marked the lines you need to pay attention to.
In line N1, the expression is checked in the condition inConfs.size() < portNum. The condition becomes falseWhen portNum <= inConfs.size(). Then in line N2 the container is accessed. inConfs. It must be accessed by indexes in the range [0 .. N – 1]. However, in the limiting case when portNum == inConfs.size()the container will go beyond the boundary, leading to undefined behavior.
A valid check should look like this:
if (portNum >= inConfs.size()) { .... }
I also swapped the operands because, in my humble opinion, this check is easier for a human to read.
After reading about this error, the reader may call the explanatory brigade: “There are no typos or copy-paste problems in this example, then what is it doing here?” The fact is that this error was propagated:
....
std::vector<PortConfig> outConfs;
....
MemoryDescPtr Node::getBaseMemDescAtOutputPort(size_t portNum) const
{
if (auto primDesc = getSelectedPrimitiveDescriptor())
{
const auto& outConfs = primDesc->getConfig().outConfs;
if (outConfs.size() < portNum) // <=
{
OPENVINO_THROW("Can't get output memory desc at port: ",
portNum, ", incorrect port number" );
}
return outConfs[portNum].getMemDesc(); // <=
}
OPENVINO_THROW("Can't get output memory desc,
primitive descriptor is not selected");
}
Analyzer warning:
V557 [CERT-ARR30-C] Array overrun is possible. The 'portNum' index is pointing beyond array bound. node.cpp 413
The code for these two functions is almost the same. It looks very much like they wrote one of them by completely copying the other, and then made minor changes: they renamed a local variable and used a different field of the object, and changed the messages when throwing exceptions. But the error also migrated to the second function.
Let's fix it too:
if (portNum >= outConfs.size()) { .... }
In general, refactoring suggests itself here: the code from these two functions should be separated into one common one and then reused.
Fragment N12
Now let's play a mini-game: you need to find typos in the code of the OpenVINO project. Well done whoever found it! Those who haven’t found it, well done too! However, I hope it's clear to everyone that this example is one big reason to use a static analyzer.
Find typos here:
template <class Key, class Value>
using caseless_unordered_map = std::unordered_map<Key, Value,
CaselessHash<Key>, CaselessEq<Key>>;
using TypeToNameMap = ov::intel_cpu::
caseless_unordered_map<std::string, Type>;
static const TypeToNameMap& get_type_to_name_tbl() {
static const TypeToNameMap type_to_name_tbl = {
{"Constant", Type::Input},
{"Parameter", Type::Input},
{"Result", Type::Output},
{"Eye", Type::Eye},
{"Convolution", Type::Convolution},
{"GroupConvolution", Type::Convolution},
{"MatMul", Type::MatMul},
{"FullyConnected", Type::FullyConnected},
{"MaxPool", Type::Pooling},
{"AvgPool", Type::Pooling},
{"AdaptiveMaxPool", Type::AdaptivePooling},
{"AdaptiveAvgPool", Type::AdaptivePooling},
{"Add", Type::Eltwise},
{"IsFinite", Type::Eltwise},
{"IsInf", Type::Eltwise},
{"IsNaN", Type::Eltwise},
{"Subtract", Type::Eltwise},
{"Multiply", Type::Eltwise},
{"Divide", Type::Eltwise},
{"SquaredDifference", Type::Eltwise},
{"Maximum", Type::Eltwise},
{"Minimum", Type::Eltwise},
{"Mod", Type::Eltwise},
{"FloorMod", Type::Eltwise},
{"Power", Type::Eltwise},
{"PowerStatic", Type::Eltwise},
{"Equal", Type::Eltwise},
{"NotEqual", Type::Eltwise},
{"Greater", Type::Eltwise},
{"GreaterEqual", Type::Eltwise},
{"Less", Type::Eltwise},
{"LessEqual", Type::Eltwise},
{"LogicalAnd", Type::Eltwise},
{"LogicalOr", Type::Eltwise},
{"LogicalXor", Type::Eltwise},
{"LogicalNot", Type::Eltwise},
{"Relu", Type::Eltwise},
{"LeakyRelu", Type::Eltwise},
{"Gelu", Type::Eltwise},
{"Elu", Type::Eltwise},
{"Tanh", Type::Eltwise},
{"Sigmoid", Type::Eltwise},
{"Abs", Type::Eltwise},
{"Sqrt", Type::Eltwise},
{"Clamp", Type::Eltwise},
{"Exp", Type::Eltwise},
{"SwishCPU", Type::Eltwise},
{"HSwish", Type::Eltwise},
{"Mish", Type::Eltwise},
{"HSigmoid", Type::Eltwise},
{"Round", Type::Eltwise},
{"PRelu", Type::Eltwise},
{"Erf", Type::Eltwise},
{"SoftPlus", Type::Eltwise},
{"SoftSign", Type::Eltwise},
{"Select", Type::Eltwise},
{"Log", Type::Eltwise},
{"BitwiseAnd", Type::Eltwise},
{"BitwiseNot", Type::Eltwise},
{"BitwiseOr", Type::Eltwise},
{"BitwiseXor", Type::Eltwise},
{"Reshape", Type::Reshape},
{"Squeeze", Type::Reshape},
{"Unsqueeze", Type::Reshape},
{"ShapeOf", Type::ShapeOf},
{"NonZero", Type::NonZero},
{"Softmax", Type::Softmax},
{"Reorder", Type::Reorder},
{"BatchToSpace", Type::BatchToSpace},
{"SpaceToBatch", Type::SpaceToBatch},
{"DepthToSpace", Type::DepthToSpace},
{"SpaceToDepth", Type::SpaceToDepth},
{"Roll", Type::Roll},
{"LRN", Type::Lrn},
{"Split", Type::Split},
{"VariadicSplit", Type::Split},
{"Concat", Type::Concatenation},
{"ConvolutionBackpropData", Type::Deconvolution},
{"GroupConvolutionBackpropData", Type::Deconvolution},
{"StridedSlice", Type::StridedSlice},
{"Slice", Type::StridedSlice},
{"Tile", Type::Tile},
{"ROIAlign", Type::ROIAlign},
{"ROIPooling", Type::ROIPooling},
{"PSROIPooling", Type::PSROIPooling},
{"DeformablePSROIPooling", Type::PSROIPooling},
{"Pad", Type::Pad},
{"Transpose", Type::Transpose},
{"LSTMCell", Type::RNNCell},
{"GRUCell", Type::RNNCell},
{"AUGRUCell", Type::RNNCell},
{"RNNCell", Type::RNNCell},
{"LSTMSequence", Type::RNNSeq},
{"GRUSequence", Type::RNNSeq},
{"AUGRUSequence", Type::RNNSeq},
{"RNNSequence", Type::RNNSeq},
{"FakeQuantize", Type::FakeQuantize},
{"BinaryConvolution", Type::BinaryConvolution},
{"DeformableConvolution", Type::DeformableConvolution},
{"TensorIterator", Type::TensorIterator},
{"Loop", Type::TensorIterator},
{"ReadValue", Type::MemoryInput}, // for construction from name
// ctor, arbitrary name is used
{"Assign", Type::MemoryOutput}, // for construction from layer ctor
{"Convert", Type::Convert},
{"NV12toRGB", Type::ColorConvert},
{"NV12toBGR", Type::ColorConvert},
{"I420toRGB", Type::ColorConvert},
{"I420toBGR", Type::ColorConvert},
{"MVN", Type::MVN},
{"NormalizeL2", Type::NormalizeL2},
{"ScatterUpdate", Type::ScatterUpdate},
{"ScatterElementsUpdate", Type::ScatterElementsUpdate},
{"ScatterNDUpdate", Type::ScatterNDUpdate},
{"Interpolate", Type::Interpolate},
{"RandomUniform", Type::RandomUniform},
{"ReduceL1", Type::Reduce},
{"ReduceL2", Type::Reduce},
{"ReduceLogicalAnd", Type::Reduce},
{"ReduceLogicalOr", Type::Reduce},
{"ReduceMax", Type::Reduce},
{"ReduceMean", Type::Reduce},
{"ReduceMin", Type::Reduce},
{"ReduceProd", Type::Reduce},
{"ReduceSum", Type::Reduce},
{"ReduceLogSum", Type::Reduce},
{"ReduceLogSumExp", Type::Reduce},
{"ReduceSumSquare", Type::Reduce},
{"Broadcast", Type::Broadcast},
{"EmbeddingSegmentsSum", Type::EmbeddingSegmentsSum},
{"EmbeddingBagPackedSum", Type::EmbeddingBagPackedSum},
{"EmbeddingBagOffsetsSum", Type::EmbeddingBagOffsetsSum},
{"Gather", Type::Gather},
{"GatherElements", Type::GatherElements},
{"GatherND", Type::GatherND},
{"GridSample", Type::GridSample},
{"OneHot", Type::OneHot},
{"RegionYolo", Type::RegionYolo},
{"ShuffleChannels", Type::ShuffleChannels},
{"DFT", Type::DFT},
{"IDFT", Type::DFT},
{"RDFT", Type::RDFT},
{"IRDFT", Type::RDFT},
{"Abs", Type::Math},
{"Acos", Type::Math},
{"Acosh", Type::Math},
{"Asin", Type::Math},
{"Asinh", Type::Math},
{"Atan", Type::Math},
{"Atanh", Type::Math},
{"Ceil", Type::Math},
{"Ceiling", Type::Math},
{"Cos", Type::Math},
{"Cosh", Type::Math},
{"Floor", Type::Math},
{"HardSigmoid", Type::Math},
{"If", Type::If},
{"Neg", Type::Math},
{"Reciprocal", Type::Math},
{"Selu", Type::Math},
{"Sign", Type::Math},
{"Sin", Type::Math},
{"Sinh", Type::Math},
{"SoftPlus", Type::Math},
{"Softsign", Type::Math},
{"Tan", Type::Math},
{"CTCLoss", Type::CTCLoss},
{"Bucketize", Type::Bucketize},
{"CTCGreedyDecoder", Type::CTCGreedyDecoder},
{"CTCGreedyDecoderSeqLen", Type::CTCGreedyDecoderSeqLen},
{"CumSum", Type::CumSum},
{"DetectionOutput", Type::DetectionOutput},
{"ExperimentalDetectronDetectionOutput",
Type::ExperimentalDetectronDetectionOutput},
{"LogSoftmax", Type::LogSoftmax},
{"TopK", Type::TopK},
{"GatherTree", Type::GatherTree},
{"GRN", Type::GRN},
{"Range", Type::Range},
{"Proposal", Type::Proposal},
{"ReorgYolo", Type::ReorgYolo},
{"ReverseSequence", Type::ReverseSequence},
{"ExperimentalDetectronTopKROIs",
Type::ExperimentalDetectronTopKROIs},
{"ExperimentalDetectronROIFeatureExtractor",
Type::ExperimentalDetectronROIFeatureExtractor},
{"ExperimentalDetectronPriorGridGenerator",
Type::ExperimentalDetectronPriorGridGenerator},
{"ExperimentalDetectronGenerateProposalsSingleImage",
Type::ExperimentalDetectronGenerateProposalsSingleImage},
{"ExtractImagePatches", Type::ExtractImagePatches},
{"GenerateProposals", Type::GenerateProposals},
{"Inverse", Type::Inverse},
{"NonMaxSuppression", Type::NonMaxSuppression},
{"NonMaxSuppressionIEInternal", Type::NonMaxSuppression},
{"NMSRotated", Type::NonMaxSuppression},
{"MatrixNms", Type::MatrixNms},
{"MulticlassNms", Type::MulticlassNms},
{"MulticlassNmsIEInternal", Type::MulticlassNms},
{"Multinomial", Type::Multinomial},
{"Reference", Type::Reference},
{"Subgraph", Type::Subgraph},
{"PriorBox", Type::PriorBox},
{"PriorBoxClustered", Type::PriorBoxClustered},
{"Interaction", Type::Interaction},
{"MHA", Type::MHA},
{"Unique", Type::Unique},
{"Ngram", Type::Ngram},
{"ScaledDotProductAttention", Type::ScaledDotProductAttention},
{"ScaledDotProductAttentionWithKVCache",
Type::ScaledDotProductAttention},
{"PagedAttentionExtension", Type::ScaledDotProductAttention},
{"RoPE", Type::RoPE},
{"GatherCompressed", Type::Gather},
{"CausalMaskPreprocess", Type::CausalMaskPreprocess},
};
return type_to_name_tbl;
}
It's funny that many, having seen the above code, will consider the task of finding errors in it an unreasonable waste of time. As a result, they will immediately scroll here to see the answer.
The funny thing is that, realizing all the inefficiency and taking into account all the labor costs, most programmers continue to manually search for errors instead of using static analysis technology.
It would seem that it could be simpler: we launch the analysis and then look at the warnings with a ready-made list of errors. There is no need to waste time looking for problem areas – they are already highlighted. All that remains is to fix it.
Analyzer warnings:
V766 An item with the same key '”Abs”' has already been added. cpu_types.cpp 178
V766 An item with the same key '”SoftPlus”' has already been added. cpu_types.cpp 198
To put it briefly and to the point, here they are – typos:
static const TypeToNameMap& get_type_to_name_tbl() {
static const TypeToNameMap type_to_name_tbl = {
....,
{"Abs", Type::Eltwise}, // <=
....,
{"SoftPlus", Type::Eltwise}, // <=
....,
{"Abs", Type::Math}, // <=
....,
{"SoftPlus", Type::Math}, // <=
....,
};
return type_to_name_tbl;
}
Obviously, there should not be identical values, and duplicates will never be used.
Conclusion
Here we have such an interesting adventure based on typos.
This was the first part of the article about checking the code of the OpenVINO project. I wish you to be more careful, and may the power of concentration be with you.
We will, of course, pass on all the information to the developers and hope that the bugs will be fixed in the near future.
I would like to remind you once again: the typos and related errors found only indicate that programmers are people too. And for happiness (and to correct errors in the code) they only need a small, but effective and their own – static code analyzer.
And, as has already happened historically, I propose try our analyzer PVS-Studio. For Open Source projects we have provided free license.
Take care of yourself and all the best!
If you want to share this article with an English-speaking audience, please use the translation link: Alexey Gorshkov. 12 moments of typos and copy-paste, or why AI hallucinates: checking OpenVINO.
