-
Notifications
You must be signed in to change notification settings - Fork 153
[JAVA_API] Fix ElementType compatibility problem introduced in 2025.1 #963
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
base: master
Are you sure you want to change the base?
[JAVA_API] Fix ElementType compatibility problem introduced in 2025.1 #963
Conversation
Breaking change was introduced in this commit: openvinotoolkit/openvino@d9c2aee ElementType.undefined was deprecated and its value was changed to be equal to ElementType.dynamic which caused a shift of the value of the remaining types.
build_jenkins |
f8e5m2(22), | ||
string(23), | ||
f4e2m1(24), | ||
f8e8m0(25); |
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.
Why new type added as PR only fix compatibility issues after deprecation of undefined?
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.
The added type has nothing to do with the fix of the problem. The problem comes from the usage of two Type_t
entries using the same int
value 0
(undefined
and dynamic
).
The fix was subtracting 1
from the values of ElementType.java
Take a look at the following example:
#include <iostream>
enum class OldType_t {
undefined, // 0
dynamic, // 1
boolean, // 2
bf16, // 3
f16, // 4
f32, // 5
f64, // 6
// etc...
};
constexpr size_t idx(OldType_t e) noexcept {
return static_cast<std::underlying_type_t<OldType_t>>(e);
}
enum class NewType_t {
dynamic, // 0
undefined = dynamic, // 0
boolean, // 1 <-- value changed
bf16, // 2 <-- value changed
f16, // 3 <-- value changed
f32, // 4 <-- value changed
f64, // 5 <-- value changed
// etc...
};
constexpr size_t idx(NewType_t e) noexcept {
return static_cast<std::underlying_type_t<NewType_t>>(e);
}
int main()
{
std::cout << "Old values:"
<<" undefined="<< idx(OldType_t::undefined)
<<" dynamic=" << idx(OldType_t::dynamic)
<<" boolean=" << idx(OldType_t::boolean)
<<" bf16=" << idx(OldType_t::bf16)
<<" f16=" << idx(OldType_t::f16)
<<" f32=" << idx(OldType_t::f32)
<<" f64=" << idx(OldType_t::f64) << std::endl;
std::cout << "New values:"
<<" undefined="<< idx(NewType_t::undefined)
<<" dynamic=" << idx(NewType_t::dynamic)
<<" boolean=" << idx(NewType_t::boolean)
<<" bf16=" << idx(NewType_t::bf16)
<<" f16=" << idx(NewType_t::f16)
<<" f32=" << idx(NewType_t::f32)
<<" f64=" << idx(NewType_t::f64) << std::endl;
return 0;
}
Old values: undefined=0 dynamic=1 boolean=2 bf16=3 f16=4 f32=5 f64=6
New values: undefined=0 dynamic=0 boolean=1 bf16=2 f16=3 f32=4 f64=5
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.
The added type has nothing to do with the fix of the problem
, this was reason for question why is added if not related to the issue.
Fix for values of ElementType.java
is OK.
Breaking change was introduced here
ElementType.undefined
was deprecated and its value was changed to be equal toElementType.dynamic
which caused a shift of the value of the remaining types.Also undo changes done to
modules/java_api/src/test/java/org/intel/openvino/CompiledModelTests.java
andmodules/java_api/src/test/java/org/intel/openvino/ModelTests.java
which changed expected types fromElementType.f32
toElementType.f16
in this commit which is incorrect since the actual type of the input and output of the model is f32