Skip to content

[AIX] test-suites failure fixes #25791

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ranjitshs
Copy link
Contributor

@ranjitshs ranjitshs commented Aug 19, 2025

Description

This PR is to fix some of the test case failures mentioned in #25790

  1. cmake/onnxruntime_unittests.cmake
    To fix the onnxruntime_shared_lib_test crash
  2. include/onnxruntime/core/providers/utils/ort_graph_to_proto.h
    For fixing , EpGraphTest.SerializeToProto_ConstantOfShape
  3. onnxruntime/core/framework/tensorprotoutils.cc
    For fixing many test failures , related to float16 or Int16 type.
  4. onnxruntime/test/framework/endian_test.cc
    For fixing, ConvertRawDataInTensorProtoTest.FloatData and ConvertRawDataInTensorProtoTest.Int32Data

Motivation and Context

To fix the AIX (Big endian) related test failures.

@ranjitshs
Copy link
Contributor Author

ranjitshs commented Aug 19, 2025

@snnn @tianleiwu @yuslepukhin
FYI.
Could you please review this PR and let me know your inputs.

Also, please trigger CI workflows for this PR.

@@ -362,11 +362,13 @@ void ConvertRawDataInTensorProto(TensorProto& tensor) {
case TensorProto_DataType_INT16:
case TensorProto_DataType_FLOAT16:
case TensorProto_DataType_BFLOAT16:
bytes = tensor.mutable_int32_data()->mutable_data();
Copy link
Member

Choose a reason for hiding this comment

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

This would be incorrect. All 4 bytes in should be swapped for a single 32 bit, not just 2.

Copy link
Contributor Author

@ranjitshs ranjitshs Aug 20, 2025

Choose a reason for hiding this comment

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

@yuslepukhin
Some scenarios, I see where sizeof() is used while setting the raw data .
If we consider those case, then we need to consider 2 only as sizeof(BFLOAT16) is 2.

  • Do we need to consider both type of handling in this method.
    One for data added by set_raw_data() and other for data added by add_int32_data types of API's.

  • Or since this method is only for handling raw data, Can we first check if raw data is present or not and return if not. Not sure why we haven't added during first time, so there could be some reason and need to test it out.

Let me know your input on this.

Below is one example of test case where we use sizeof() for setting the raw data.

 template <typename T>
void TestInitializerRawData() {
  std::vector<T> data = GetInitializerData<T>();

  ONNX_NAMESPACE::TensorProto tensor_proto;
  tensor_proto.set_data_type(GetTensorProtoDataType<T>());
  tensor_proto.set_name("OptimizerInitializerTest_RawData");
  tensor_proto.add_dims(3);
  tensor_proto.add_dims(4);
  utils::SetRawDataInTensorProto(tensor_proto, data.data(), data.size() * sizeof(T));

  const Initializer init(tensor_proto, std::filesystem::path());

  for (size_t idx = 0; idx < data.size(); idx++) {
    EXPECT_EQ(data[idx], init.data<T>()[idx]);
  }
}

TEST(OptimizerInitializerTest, RawData) {
  TestInitializerRawData<int8_t>();
  TestInitializerRawData<uint8_t>();
  TestInitializerRawData<int32_t>();
  TestInitializerRawData<int64_t>();
  TestInitializerRawData<MLFloat16>();
  TestInitializerRawData<BFloat16>();
  TestInitializerRawData<float>();
  TestInitializerRawData<double>();
}

Copy link
Member

@yuslepukhin yuslepukhin Aug 20, 2025

Choose a reason for hiding this comment

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

If the data is stored as raw data, and the size of the type is 2 bytes, then they are stored contigiously and for that sizeof() needs to be 2 bytes.
However, if it is not stored as raw data, and in this case stored in int32, then all 4 bytes need to be swapped, although only two bytes are meaningful.
Thus we either need two different switches, branch on the presense of raw data and set the sizeof() accordingly OR we need two different sizes for raw data and otherwise.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay. I will put two different switches based on raw data presence. Let me test it out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants