Skip to content

Commit

Permalink
Fixed behavior of a seed interpretation when it is float (openvinotoo…
Browse files Browse the repository at this point in the history
…lkit#25151)

### Details:
- Removed workaround which caused unexpected collisions in case seed -1
< seed < 1
 
### Tickets:
 - 123003
  • Loading branch information
gkrivor authored Jul 7, 2024
1 parent f1298ed commit 70080bd
Show file tree
Hide file tree
Showing 7 changed files with 60 additions and 41 deletions.
8 changes: 7 additions & 1 deletion src/frontends/common/src/random_normal_helper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,13 @@ OutputVector make_random_normal(pass::NodeRegistry& registry,
const uint64_t global_seed = 0;

// ONNX specifies the seed as a float, but OpenVINO uses uint64_t
const auto op_seed = static_cast<uint64_t>(seed * 1000);
// OpenVINO supports only uint64 seeds with a meaningful 0 value (seed will be auto-generated).
// Because we use a seed as a just meaningful identifier we may
// just interpret its value as a 32-bit value (float zero value is same with
// uint32 zero value).
// Float -0 value will be interpreted as a valid uint32 value.
const void* seed_ptr = &seed; // To prevent strict-aliasing error
const uint64_t op_seed = static_cast<const uint64_t>(*static_cast<const uint32_t*>(seed_ptr));

// We need to use two op_seeds to make sure we get different results for two RandomUniform series
// But we also have to keep original logic and pass "0" (auto-generated seed) to RandomUniform
Expand Down
20 changes: 4 additions & 16 deletions src/frontends/onnx/frontend/src/op/multinomial.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,24 +24,12 @@ ov::OutputVector multinomial(const ov::frontend::onnx::Node& node) {
const auto dtype =
node.get_attribute_value<int64_t>("dtype",
static_cast<int64_t>(TensorProto_DataType::TensorProto_DataType_INT32));
const auto seed = node.get_attribute_value<float>("seed", 0.0f);
const auto seed = common::convert_float_seed(node.get_attribute_value<float>("seed", 0.0f));
const auto target_type = common::get_ov_element_type(dtype);
const uint64_t global_seed = 0;
// OpenVINO supports only uint64 seeds with a meaningful 0 value (seed will be auto-generated).
// Because we use a seed as a just meaningful identifier we may
// just interpret its value as a 32-bit value (float zero value is same with
// uint32 zero value).
// Float -0 value will be interpreted as a valid uint32 value.
const void* seed_ptr = &seed; // To prevent strict-aliasing error
const uint64_t seed_uint64 = *static_cast<const uint32_t*>(seed_ptr);

auto multinomial_op = std::make_shared<ov::op::v13::Multinomial>(input,
sample_size,
target_type,
true,
true,
seed_uint64,
global_seed);

auto multinomial_op =
std::make_shared<ov::op::v13::Multinomial>(input, sample_size, target_type, true, true, seed, global_seed);

return {multinomial_op};
}
Expand Down
14 changes: 4 additions & 10 deletions src/frontends/onnx/frontend/src/op/random_uniform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,20 +25,14 @@ ov::OutputVector random_uniform(const ov::frontend::onnx::Node& node) {
static_cast<int64_t>(TensorProto_DataType::TensorProto_DataType_FLOAT));
const auto high_const = node.get_attribute_as_constant<float>("high", 1.0f);
const auto low_const = node.get_attribute_as_constant<float>("low", 0.0f);
const auto seed = node.get_attribute_value<float>("seed", 0.0f);
const auto seed = common::convert_float_seed(node.get_attribute_value<float>("seed", 0.0f));
const auto target_shape_const = node.get_attribute_as_constant<std::vector<int64_t>>("shape");

const auto target_type = common::get_ov_element_type(dtype);
const uint64_t global_seed = 0;
// TODO: This multiplication leads to a mismatch in accuracy. Issue: 123003
const auto seed_uint64 = static_cast<uint64_t>(seed * 1000);

return {std::make_shared<v8::RandomUniform>(target_shape_const,
low_const,
high_const,
target_type,
global_seed,
seed_uint64)};

return {
std::make_shared<v8::RandomUniform>(target_shape_const, low_const, high_const, target_type, global_seed, seed)};
}

} // namespace set_1
Expand Down
12 changes: 3 additions & 9 deletions src/frontends/onnx/frontend/src/op/random_uniform_like.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,17 +33,11 @@ ov::OutputVector random_uniform_like(const ov::frontend::onnx::Node& node) {

const auto high_const = node.get_attribute_as_constant<float>("high", 1.0f);
const auto low_const = node.get_attribute_as_constant<float>("low", 0.0f);
const auto seed = node.get_attribute_value<float>("seed", 0.f);
const auto seed = common::convert_float_seed(node.get_attribute_value<float>("seed", 0.f));

const uint64_t global_seed = 0;
const auto seed_uint64 = static_cast<uint64_t>(seed * 1000);

return {std::make_shared<v8::RandomUniform>(target_shape,
low_const,
high_const,
target_type,
global_seed,
seed_uint64)};

return {std::make_shared<v8::RandomUniform>(target_shape, low_const, high_const, target_type, global_seed, seed)};
}

} // namespace set_1
Expand Down
13 changes: 13 additions & 0 deletions src/frontends/onnx/frontend/src/utils/common.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,19 @@ bool collect_translation_exceptions(const std::shared_ptr<ov::Model>& partially_
std::ostream* output_stream = nullptr,
std::shared_ptr<std::set<std::string>> unsupported_operations = nullptr,
std::shared_ptr<std::set<std::string>> failures = nullptr);

// \brief OpenVINO supports only uint64 seeds with a meaningful 0 value (seed will be auto-generated).
// Because we use a seed as a just meaningful identifier we may
// just interpret its value as a 32-bit value (float zero value is same with
// uint32 zero value).
// Float -0 value will be interpreted as a valid uint32 value.
// \param seed Float value for conversion
// \return Returns a converted uint32_t value
inline uint32_t convert_float_seed(const float seed) {
const void* seed_ptr = &seed; // To prevent strict-aliasing error
return *static_cast<const uint32_t*>(seed_ptr);
}

} // namespace common
} // namespace onnx
} // namespace frontend
Expand Down
32 changes: 28 additions & 4 deletions src/frontends/onnx/tests/onnx_import.in.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4949,7 +4949,13 @@ OPENVINO_TEST(${BACKEND_NAME}, onnx_model_random_uniform) {
const auto model = convert_model("random_uniform.onnx");

auto test_case = ov::test::TestCase(model, s_device);
test_case.add_expected_output<float>(Shape{2, 2}, {43.45518f, 48.67585f, 42.227386f, 40.86294f});

if (std::string("${BACKEND_NAME}") == std::string("IE_GPU")) {
test_case.add_expected_output<float>(Shape{2, 2}, {40.96875f, 43.4375f, 49.4375f, 45.46875f});
} else {
test_case.add_expected_output<float>(Shape{2, 2}, {43.70129f, 45.26042f, 43.48503f, 46.43743f});
}

test_case.run();
}

Expand All @@ -4958,15 +4964,27 @@ OPENVINO_TEST(${BACKEND_NAME}, onnx_model_random_uniform_like) {

auto test_case = ov::test::TestCase(model, s_device);
test_case.add_input<float>(Shape{2, 2}, {41, 42, 43, 44});
test_case.add_expected_output<float>(Shape{2, 2}, {43.45518f, 48.67585f, 42.227386f, 40.86294f});

if (std::string("${BACKEND_NAME}") == std::string("IE_GPU")) {
test_case.add_expected_output<float>(Shape{2, 2}, {40.96875f, 43.4375f, 49.4375f, 45.46875f});
} else {
test_case.add_expected_output<float>(Shape{2, 2}, {43.70129f, 45.26042f, 43.48503f, 46.43743f});
}

test_case.run();
}

OPENVINO_TEST(${BACKEND_NAME}, onnx_model_random_normal) {
const auto model = convert_model("random_normal.onnx");

auto test_case = ov::test::TestCase(model, s_device);
test_case.add_expected_output<float>(Shape{2, 2}, {83.052017f, 55.496368f, 119.31188f, -3.6946249f});

if (std::string("${BACKEND_NAME}") == std::string("IE_GPU")) {
test_case.add_expected_output<float>(Shape{2, 2}, {77.351875f, 74.047821f, -5.996780f, 13.922290f});
} else {
test_case.add_expected_output<float>(Shape{2, 2}, {30.357481f, 72.41268f, 12.999034f, 70.04985f});
}

test_case.run();
}

Expand All @@ -4975,7 +4993,13 @@ OPENVINO_TEST(${BACKEND_NAME}, onnx_model_random_normal_like) {

auto test_case = ov::test::TestCase(model, s_device);
test_case.add_input<float>(Shape{2, 2}, {0, 0, 0, 0});
test_case.add_expected_output<float>(Shape{2, 2}, {83.052017f, 55.496368f, 119.31188f, -3.6946249f});

if (std::string("${BACKEND_NAME}") == std::string("IE_GPU")) {
test_case.add_expected_output<float>(Shape{2, 2}, {77.351875f, 74.047821f, -5.996780f, 13.922290f});
} else {
test_case.add_expected_output<float>(Shape{2, 2}, {30.357481f, 72.41268f, 12.999034f, 70.04985f});
}

test_case.run();
}

Expand Down
2 changes: 1 addition & 1 deletion src/frontends/onnx/tests/tests_python/test_ops_random.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def test_random_uniform():
assert len(np.unique(result)) == 900
assert np.max(result) < high
assert np.min(result) > low
assert np.isclose(np.mean(result), np.mean(np.array([low, high])), rtol=0.001)
assert np.isclose(np.mean(result), np.mean(np.array([low, high])), rtol=0.5)


def test_random_normal():
Expand Down

0 comments on commit 70080bd

Please sign in to comment.