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.

Sign up for GitHub

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

Test calculation output shape for Broadcast op, relax restrictions for partially dynamic input data #1247

Merged
merged 17 commits into from
Aug 10, 2020
Merged

Test calculation output shape for Broadcast op, relax restrictions for partially dynamic input data #1247

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
0d55329
added tests to explicit mode
Jul 6, 2020
e271057
extend calculation output shape for Broadcast numpy mode + unit tests
Jul 7, 2020
351a6eb
extended calculation output shape for bidirectional mode
Jul 7, 2020
b43ab35
fixed checking shape in bidirectional mode
Jul 7, 2020
f53569e
code review remarks introduced
Jul 23, 2020
e6f006a
fixed error message
Jul 23, 2020
b66fc7a
Merge remote-tracking branch 'upstream/master' into mbencer/Broadcast…
Jul 23, 2020
d3fe5a1
Apply suggestions from code review
Jul 27, 2020
d75b22a
fix test
Jul 28, 2020
4c8eff0
Merge remote-tracking branch 'upstream/master' into mbencer/Broadcast…
Jul 28, 2020
a6f30eb
fixed tests
Jul 28, 2020
b246bf7
Merge remote-tracking branch 'upstream/master' into mbencer/Broadcast…
Jul 29, 2020
c46c478
Added unit tests after code review
Jul 29, 2020
1e80fa0
Merge remote-tracking branch 'upstream/master' into mbencer/Broadcast…
Aug 5, 2020
921f29b
Merge remote-tracking branch 'upstream/master' into mbencer/Broadcast…
Aug 6, 2020
e34c505
relax biderectional mode
Aug 7, 2020
d42e0fe
Merge remote-tracking branch 'upstream/master' into mbencer/Broadcast…
Aug 7, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 25 additions & 12 deletions ngraph/src/ngraph/op/broadcast.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,15 +88,21 @@ std::pair<bool, AxisSet> op::v3::Broadcast::get_broadcast_axes() const

namespace
{
PartialShape
get_result_shape_bidirectional(Node* this_ptr, Shape& arg_shape, Shape& target_shape)
PartialShape get_result_shape_bidirectional(Node* this_ptr,
const PartialShape& arg_shape,
Shape& target_shape)
lazarevevgeny marked this conversation as resolved.
Show resolved Hide resolved
{
if (arg_shape.rank().is_dynamic())
{
return PartialShape::dynamic();
}
auto arg_shape_vec = static_cast<std::vector<Dimension>>(arg_shape);
PartialShape result_shape;
// Add left padding to shorter target or argument shape
const auto target_padded_rank = std::max(arg_shape.size(), target_shape.size());
while (arg_shape.size() < target_padded_rank)
const auto target_padded_rank = std::max(arg_shape_vec.size(), target_shape.size());
while (arg_shape_vec.size() < target_padded_rank)
{
arg_shape.insert(arg_shape.begin(), 1);
arg_shape_vec.insert(arg_shape_vec.begin(), 1);
}
while (target_shape.size() < target_padded_rank)
{
Expand All @@ -106,15 +112,21 @@ namespace
result_shape = target_shape;
for (auto i = 0; i < target_shape.size(); ++i)
{
if (arg_shape_vec[i].is_dynamic())
{
result_shape[i] = Dimension::dynamic();
continue;
}
const size_t arg_shape_dim = arg_shape_vec[i].get_length();
NODE_VALIDATION_CHECK(this_ptr,
arg_shape[i] == 1 || target_shape[i] == 1 ||
arg_shape[i] == target_shape[i],
arg_shape_dim == 1 || target_shape[i] == 1 ||
arg_shape_dim == target_shape[i],
"Broadcast incorrect target shape. Expecting either 1 or ",
arg_shape[i],
arg_shape_dim,
". Got ",
target_shape[i]);

result_shape[i] = std::max(arg_shape[i], target_shape[i]);
result_shape[i] = std::max(arg_shape_dim, target_shape[i]);
}
return result_shape;
}
Expand All @@ -141,9 +153,9 @@ void op::v3::Broadcast::validate_and_infer_types()
auto result_shape = get_output_partial_shape(0);
if (m_mode.m_type == BroadcastType::BIDIRECTIONAL)
{
if (get_input_partial_shape(0).is_static() && get_input_partial_shape(1).is_static())
if (get_input_partial_shape(0).rank().is_static() && get_input_partial_shape(1).is_static())
{
auto arg_shape = get_input_shape(0);
auto arg_shape = get_input_partial_shape(0);

const auto shape_constant =
as_type_ptr<op::v0::Constant>(input_value(1).get_node_shared_ptr());
Expand Down Expand Up @@ -192,7 +204,8 @@ bool op::v3::Broadcast::evaluate(const HostTensorVector& outputs, const HostTens
{
auto arg_shape = inputs[0]->get_shape();
Shape target_shape = op::util::BroadcastBase::get_target_shape(inputs[1]);
PartialShape result_shape = get_result_shape_bidirectional(this, arg_shape, target_shape);
PartialShape result_shape =
get_result_shape_bidirectional(this, PartialShape{arg_shape}, target_shape);
auto pair_broadcast_axes =
get_broadcast_axes_bidirectional(arg_shape, result_shape.to_shape());
return op::util::BroadcastBase::evaluate_broadcast(
Expand Down
54 changes: 40 additions & 14 deletions ngraph/src/ngraph/op/util/broadcast_base.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,28 +46,40 @@ op::util::BroadcastBase::BroadcastBase(const Output<Node>& arg,
}

PartialShape op::util::BroadcastBase::get_result_shape_numpy_pdpd(
const Shape& arg0_shape, const Shape& target_shape, const op::BroadcastModeSpec& broadcast_spec)
const PartialShape& arg0_shape,
const Shape& target_shape,
const op::BroadcastModeSpec& broadcast_spec)
{
if (arg0_shape.rank().is_dynamic())
{
return PartialShape::dynamic(target_shape.size());
}
const auto arg_rank_length = arg0_shape.rank().get_length();
PartialShape result_shape = target_shape;
auto start_axis = (broadcast_spec.m_type == op::BroadcastType::PDPD)
? broadcast_spec.m_axis
: target_shape.size() - arg0_shape.size();
: target_shape.size() - arg_rank_length;
NODE_VALIDATION_CHECK(this,
start_axis >= 0,
"Broadcast target_shape has smaller rank ",
target_shape.size(),
" than arg shape ",
arg0_shape.size());
arg_rank_length);
for (auto i = start_axis; i < target_shape.size(); i++)
{
if (arg0_shape[i - start_axis].is_dynamic())
{
result_shape[i] = Dimension::dynamic();
continue;
}
const size_t arg_dim = arg0_shape[i - start_axis].get_length();
NODE_VALIDATION_CHECK(this,
arg0_shape[i - start_axis] == 1 || target_shape[i] == 1 ||
arg0_shape[i - start_axis] == target_shape[i],
arg_dim == 1 || target_shape[i] == 1 || arg_dim == target_shape[i],
"Broadcast incorrect target shape. Expecting either 1 or ",
arg0_shape[i - start_axis],
arg_dim,
" . Got ",
target_shape[i]);
result_shape[i] = std::max(arg0_shape[i - start_axis], target_shape[i]);
result_shape[i] = std::max(arg_dim, target_shape[i]);
}
return result_shape;
}
Expand Down Expand Up @@ -138,13 +150,27 @@ void op::util::BroadcastBase::validate_and_infer_types()
}

PartialShape result_shape{PartialShape::dynamic()};
auto input_rank = input_value(0).get_partial_shape().rank();
auto output_rank = input_value(1).get_partial_shape();
if (input_rank.is_static() && output_rank.is_static() && output_rank[0].is_static())
const auto input_rank = input_value(0).get_partial_shape().rank();
const auto output_shape = input_value(1).get_partial_shape();
const bool is_output_rank_static =
output_shape.rank().is_static() && output_shape[0].is_static();
Copy link
Contributor

Choose a reason for hiding this comment

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

The variable name is misleading. The conditions are more restrictive than the variable name. In fact they boil down to output_shape.is_static() - since you've already enforced 1D target shape and is_static function checks whether partial shape rank is static as well as all of its dimensions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right, I changed it to is_target_shape_known


if (m_mode.m_type == BroadcastType::BIDIRECTIONAL)
{
if (input_rank.is_static() && is_output_rank_static)
{
result_shape = PartialShape::dynamic(
std::max(input_rank.get_length(), output_shape[0].get_length()));
}
}
else
{
result_shape =
PartialShape::dynamic(std::max(input_rank.get_length(), output_rank[0].get_length()));
if (is_output_rank_static)
{
result_shape = PartialShape::dynamic(output_shape[0].get_length());
}
}

const auto shape_constant = as_type_ptr<op::v0::Constant>(input_value(1).get_node_shared_ptr());

if (auto concat = as_type_ptr<op::v0::Concat>(input_value(1).get_node_shared_ptr()))
Expand Down Expand Up @@ -204,9 +230,9 @@ void op::util::BroadcastBase::validate_and_infer_types()
}
else if (m_mode.m_type == BroadcastType::NUMPY || m_mode.m_type == BroadcastType::PDPD)
{
if (get_input_partial_shape(0).is_static() && get_input_partial_shape(1).is_static())
if (get_input_partial_shape(0).rank().is_static() && get_input_partial_shape(1).is_static())
arogowie-intel marked this conversation as resolved.
Show resolved Hide resolved
{
auto arg_shape = get_input_shape(0);
const auto& arg_shape = get_input_partial_shape(0);

if (shape_constant)
{
Expand Down
2 changes: 1 addition & 1 deletion ngraph/src/ngraph/op/util/broadcast_base.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ namespace ngraph
const AxisSet& broadcast_axes);

PartialShape
get_result_shape_numpy_pdpd(const Shape& arg0_shape,
get_result_shape_numpy_pdpd(const PartialShape& arg0_shape,
const Shape& target_shape,
const op::BroadcastModeSpec& broadcast_spec);
static std::pair<bool, AxisSet>
Expand Down
Loading