-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
[Opset13][FP8] Introduce FakeConvert op core #20930
Changes from 17 commits
82649ed
e34026b
c75b12e
1ff36f3
ccc4c86
b064102
a7aa173
e39f16f
1430dc9
f444057
b821657
be23a19
849c410
9474401
22160f6
821f5f4
e36c41b
213f45e
a51f998
42e67aa
04d706e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
// Copyright (C) 2018-2023 Intel Corporation | ||
// SPDX-License-Identifier: Apache-2.0 | ||
// | ||
|
||
#pragma once | ||
|
||
#include "openvino/op/op.hpp" | ||
|
||
namespace ov { | ||
namespace op { | ||
namespace v13 { | ||
/// \ingroup ov_ops_cpp_api | ||
class OPENVINO_API FakeConvert : public Op { | ||
public: | ||
OPENVINO_OP("FakeConvert", "opset13"); | ||
|
||
FakeConvert() = default; | ||
FakeConvert(const ov::Output<ov::Node>& arg, | ||
const ov::Output<ov::Node>& scale, | ||
const ov::Output<ov::Node>& shift, | ||
std::string destination_type = "f8e4m3", | ||
bool apply_scale = false); | ||
|
||
void validate_and_infer_types() override; | ||
std::shared_ptr<ov::Node> clone_with_new_inputs(const ov::OutputVector& new_args) const override; | ||
bool visit_attributes(ov::AttributeVisitor& visitor) override; | ||
bool has_evaluate() const override; | ||
|
||
bool get_apply_scale() const; | ||
const std::string& get_destination_type() const; | ||
|
||
private: | ||
void validate_type() const; | ||
|
||
std::string m_destination_type = "f8e4m3"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this string? Should be ov::element::Type. See There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Requires adding new types here: https://github.com/openvinotoolkit/openvino/blob/master/src/core/include/openvino/core/type/element_type.hpp#L35-L55 |
||
bool m_apply_scale = false; | ||
}; | ||
} // namespace v13 | ||
} // namespace op | ||
} // namespace ov |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,74 @@ | ||
// Copyright (C) 2018-2022 Intel Corporation | ||
// SPDX-License-Identifier: Apache-2.0 | ||
// | ||
|
||
#include "openvino/op/fake_convert.hpp" | ||
|
||
#include "itt.hpp" | ||
|
||
namespace ov { | ||
namespace op { | ||
namespace v13 { | ||
namespace fake_convert { | ||
static const std::vector<std::string>& get_valid_types() { | ||
static const std::vector<std::string> valid_types{"f8e4m3", "f8e5m2"}; | ||
return valid_types; | ||
} | ||
} // namespace fake_convert | ||
FakeConvert::FakeConvert(const ov::Output<ov::Node>& arg, | ||
const ov::Output<ov::Node>& scale, | ||
const ov::Output<ov::Node>& shift, | ||
std::string destination_type, | ||
bool apply_scale) | ||
: Op({arg, scale, shift}), | ||
m_destination_type(std::move(destination_type)), | ||
m_apply_scale(apply_scale) { | ||
constructor_validate_and_infer_types(); | ||
} | ||
|
||
bool FakeConvert::get_apply_scale() const { | ||
return m_apply_scale; | ||
} | ||
|
||
const std::string& FakeConvert::get_destination_type() const { | ||
return m_destination_type; | ||
} | ||
|
||
void FakeConvert::validate_and_infer_types() { | ||
OV_OP_SCOPE(v13_FakeConvert_validate_and_infer_types); | ||
validate_type(); | ||
set_output_type(0, get_input_element_type(0), get_input_partial_shape(0)); | ||
} | ||
|
||
std::shared_ptr<ov::Node> FakeConvert::clone_with_new_inputs(const ov::OutputVector& new_args) const { | ||
OV_OP_SCOPE(v13_FakeConvert_clone_with_new_inputs); | ||
OPENVINO_ASSERT(new_args.size() == 3, "Incorrect number of new arguments"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As scale and shift are not mandatory, please make them optional. Otherwise I don't understand why we have There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you make both There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's make the shift an optional parameter. I would make scale as required. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that we always need to apply scale, but shift can be optional. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As discussed, in the updated version |
||
|
||
return std::make_shared<FakeConvert>(new_args.at(0), | ||
new_args.at(1), | ||
new_args.at(2), | ||
m_destination_type, | ||
m_apply_scale); | ||
} | ||
|
||
bool FakeConvert::visit_attributes(ov::AttributeVisitor& visitor) { | ||
OV_OP_SCOPE(v13_FakeConvert_visit_attributes); | ||
visitor.on_attribute("destination_type", m_destination_type); | ||
visitor.on_attribute("apply_scale", m_apply_scale); | ||
|
||
return true; | ||
} | ||
|
||
void FakeConvert::validate_type() const { | ||
const auto& valid_types = fake_convert::get_valid_types(); | ||
OPENVINO_ASSERT(std::find(valid_types.begin(), valid_types.end(), m_destination_type) != valid_types.end(), | ||
"Bad format for f8 conversion type: " + m_destination_type); | ||
} | ||
|
||
bool FakeConvert::has_evaluate() const { | ||
return false; | ||
} | ||
|
||
} // namespace v13 | ||
} // namespace op | ||
} // namespace ov |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
// Copyright (C) 2018-2023 Intel Corporation | ||
// SPDX-License-Identifier: Apache-2.0 | ||
// | ||
|
||
#include "openvino/op/fake_convert.hpp" | ||
|
||
#include <gtest/gtest.h> | ||
|
||
#include "common_test_utils/type_prop.hpp" | ||
|
||
using namespace ov; | ||
using ov::op::v0::Parameter; | ||
|
||
TEST(type_prop, fake_convert_basic_f32) { | ||
const auto data = std::make_shared<Parameter>(element::f32, PartialShape{2, 3, 8, 6}); | ||
const auto scale = std::make_shared<Parameter>(element::f32, PartialShape{}); | ||
const auto shift = std::make_shared<Parameter>(element::f32, PartialShape{}); | ||
|
||
const auto op = std::make_shared<op::v13::FakeConvert>(data, scale, shift); | ||
EXPECT_EQ(op->get_output_element_type(0), element::f32); | ||
EXPECT_EQ(op->get_output_partial_shape(0), (PartialShape{2, 3, 8, 6})); | ||
} | ||
|
||
TEST(type_prop, fake_convert_basic_f16) { | ||
const auto data = std::make_shared<Parameter>(element::f16, PartialShape{2, 3, 8, 6}); | ||
const auto scale = std::make_shared<Parameter>(element::f16, PartialShape{}); | ||
const auto shift = std::make_shared<Parameter>(element::f16, PartialShape{}); | ||
|
||
const auto op = std::make_shared<op::v13::FakeConvert>(data, scale, shift); | ||
EXPECT_EQ(op->get_output_element_type(0), element::f16); | ||
EXPECT_EQ(op->get_output_partial_shape(0), (PartialShape{2, 3, 8, 6})); | ||
} | ||
|
||
TEST(type_prop, fake_convert_dynamic_shape) { | ||
const auto data = std::make_shared<Parameter>(element::f32, PartialShape::dynamic()); | ||
const auto scale = std::make_shared<Parameter>(element::f32, PartialShape{}); | ||
const auto shift = std::make_shared<Parameter>(element::f32, PartialShape{}); | ||
|
||
const auto op = std::make_shared<op::v13::FakeConvert>(data, scale, shift); | ||
EXPECT_EQ(op->get_output_element_type(0), element::f32); | ||
EXPECT_EQ(op->get_output_partial_shape(0), (PartialShape::dynamic())); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
// Copyright (C) 2018-2023 Intel Corporation | ||
// SPDX-License-Identifier: Apache-2.0 | ||
// | ||
|
||
#include "openvino/op/fake_convert.hpp" | ||
|
||
#include <gtest/gtest.h> | ||
|
||
#include "visitors/visitors.hpp" | ||
|
||
using ov::Shape; | ||
using ov::op::v0::Parameter; | ||
using ov::test::NodeBuilder; | ||
|
||
TEST(attributes, fake_convert_v13_attributes_default) { | ||
using ov::op::v13::FakeConvert; | ||
NodeBuilder::get_ops().register_factory<FakeConvert>(); | ||
const auto data = std::make_shared<Parameter>(ov::element::f32, ov::PartialShape{2, 3, 8, 6}); | ||
const auto scale = std::make_shared<Parameter>(ov::element::f32, ov::PartialShape{}); | ||
const auto shift = std::make_shared<Parameter>(ov::element::f32, ov::PartialShape{}); | ||
|
||
const auto op = std::make_shared<FakeConvert>(data, scale, shift); | ||
|
||
NodeBuilder builder(op, {data, scale, shift}); | ||
auto g_op = ov::as_type_ptr<FakeConvert>(builder.create()); | ||
|
||
EXPECT_EQ(g_op->get_apply_scale(), op->get_apply_scale()); | ||
EXPECT_EQ(g_op->get_destination_type(), op->get_destination_type()); | ||
EXPECT_EQ(g_op->get_output_element_type(0), op->get_output_element_type(0)); | ||
EXPECT_EQ(g_op->get_output_partial_shape(0), op->get_output_partial_shape(0)); | ||
} | ||
|
||
TEST(attributes, fake_convert_v13_attributes_custom) { | ||
using ov::op::v13::FakeConvert; | ||
NodeBuilder::get_ops().register_factory<FakeConvert>(); | ||
const auto data = std::make_shared<Parameter>(ov::element::f32, ov::PartialShape{2, 3, 8, 6}); | ||
const auto scale = std::make_shared<Parameter>(ov::element::f32, ov::PartialShape{}); | ||
const auto shift = std::make_shared<Parameter>(ov::element::f32, ov::PartialShape{}); | ||
|
||
const auto op = std::make_shared<FakeConvert>(data, scale, shift, "f8e5m2", true); | ||
|
||
NodeBuilder builder(op, {data, scale, shift}); | ||
auto g_op = ov::as_type_ptr<FakeConvert>(builder.create()); | ||
|
||
EXPECT_EQ(g_op->get_apply_scale(), op->get_apply_scale()); | ||
EXPECT_EQ(g_op->get_destination_type(), op->get_destination_type()); | ||
EXPECT_EQ(g_op->get_output_element_type(0), op->get_output_element_type(0)); | ||
EXPECT_EQ(g_op->get_output_partial_shape(0), op->get_output_partial_shape(0)); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No reason to have
apply_scale
set to false by default, suggesting to set it automatically in case if scale input is provided (and sale and shift inputs are made optional as suggested in another comment). If one wants to provide shift but doesn't apply scale, then as we don't have "gaps" in the list of operation arguments, one should provide arbitrary scale (will be ignored), then shift and set apply_scale to false. This is only (and looks like very rare situation) when apply_scale should have manual control as a parameter of constructor.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FakeConvert(input) ==> apply_scale = false (as no scale provided)
FakeConvert(input, scale) ==> apply_scale = true
FakeConvert(input, scale, shift) ==> apply_scale = true
FakeConvert(input, scale, shift, "f8e4m", false) ==> apply_scale = false, scale input is ignored
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's why I started the conversation above: #20930 (comment)
So the final suggestion is to have three constructors and the
apply_scale
attribute.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed, in the updated version
scale
input is required andshift
input optional.The
apply_scale
attribute has been removed.