From 7964d3aef6038ea82b0982ec5a520b5708c8a136 Mon Sep 17 00:00:00 2001 From: Edward Chen <18449977+edgchen1@users.noreply.github.com> Date: Fri, 18 Oct 2024 09:26:06 -0700 Subject: [PATCH] Specify iOS simulator runtime version (#22474) - Allow specification of iOS simulator runtime version to use. - Pick simulator runtime version (iphonesimulator 16.4) that is supported by the Xcode version (14.3.1) that we use. - Disable CoreML EP's DepthToSpace op support for CoreML version less than 7, with DCR mode, and FP16 input. It doesn't produce the correct output in this case. - Some cleanup of iOS test infrastructure. --- cmake/onnxruntime_unittests.cmake | 9 +++- .../builders/impl/depthtospace_op_builder.cc | 14 ++++++ .../test/logging_apis/test_logging_apis.cc | 8 +++- onnxruntime/test/unittest_main/test_main.cc | 10 +++-- onnxruntime/test/xctest/xcgtest.mm | 43 ++++++++++++++++--- .../github/apple/get_simulator_device_info.py | 29 +++++++++---- .../azure-pipelines/mac-ios-ci-pipeline.yml | 8 ++++ .../stages/mac-ios-packaging-build-stage.yml | 9 ++++ 8 files changed, 108 insertions(+), 22 deletions(-) diff --git a/cmake/onnxruntime_unittests.cmake b/cmake/onnxruntime_unittests.cmake index a2495de5df..cbae6990cd 100644 --- a/cmake/onnxruntime_unittests.cmake +++ b/cmake/onnxruntime_unittests.cmake @@ -134,9 +134,14 @@ function(AddTest) if (IOS) # target_sources(${_UT_TARGET} PRIVATE ${TEST_SRC_DIR}/xctest/orttestmain.m) + + set(_UT_IOS_BUNDLE_GUI_IDENTIFIER com.onnxruntime.utest.${_UT_TARGET}) + # replace any characters that are not valid in a bundle identifier with '-' + string(REGEX REPLACE "[^a-zA-Z0-9\\.-]" "-" _UT_IOS_BUNDLE_GUI_IDENTIFIER ${_UT_IOS_BUNDLE_GUI_IDENTIFIER}) + set_target_properties(${_UT_TARGET} PROPERTIES FOLDER "ONNXRuntimeTest" MACOSX_BUNDLE_BUNDLE_NAME ${_UT_TARGET} - MACOSX_BUNDLE_GUI_IDENTIFIER com.onnxruntime.utest.${_UT_TARGET} + MACOSX_BUNDLE_GUI_IDENTIFIER ${_UT_IOS_BUNDLE_GUI_IDENTIFIER} MACOSX_BUNDLE_LONG_VERSION_STRING ${ORT_VERSION} MACOSX_BUNDLE_BUNDLE_VERSION ${ORT_VERSION} MACOSX_BUNDLE_SHORT_VERSION_STRING ${ORT_VERSION} @@ -163,7 +168,7 @@ function(AddTest) set_target_properties(${_UT_TARGET}_xc PROPERTIES FOLDER "ONNXRuntimeXCTest" MACOSX_BUNDLE_BUNDLE_NAME ${_UT_TARGET}_xc - MACOSX_BUNDLE_GUI_IDENTIFIER com.onnxruntime.utest.${_UT_TARGET} + MACOSX_BUNDLE_GUI_IDENTIFIER ${_UT_IOS_BUNDLE_GUI_IDENTIFIER} MACOSX_BUNDLE_LONG_VERSION_STRING ${ORT_VERSION} MACOSX_BUNDLE_BUNDLE_VERSION ${ORT_VERSION} MACOSX_BUNDLE_SHORT_VERSION_STRING ${ORT_VERSION} diff --git a/onnxruntime/core/providers/coreml/builders/impl/depthtospace_op_builder.cc b/onnxruntime/core/providers/coreml/builders/impl/depthtospace_op_builder.cc index ddaa19c7fa..fec14dfd09 100644 --- a/onnxruntime/core/providers/coreml/builders/impl/depthtospace_op_builder.cc +++ b/onnxruntime/core/providers/coreml/builders/impl/depthtospace_op_builder.cc @@ -145,6 +145,20 @@ bool DepthToSpaceOpBuilder::IsOpSupportedImpl(const Node& node, const OpBuilderI LOGS(logger, VERBOSE) << "DepthToSpace: CRD mode requires static shape"; return false; } + + if (mode == "DCR" && input_params.coreml_version < 7) { + int32_t input_type = ONNX_NAMESPACE::TensorProto_DataType_UNDEFINED; + GetType(*input_defs[0], input_type, logger); + + if (input_type == ONNX_NAMESPACE::TensorProto_DataType_FLOAT16) { + // In CoreML version 6 (e.g., on an iOS 16 simulator) with DCR mode and float16 input, the output is all zeros + // in this unit test: TensorOpTest/1.DepthToSpaceTest_4. + // However, CoreML version 7 is fine. + // Don't support CoreML version < 7, DCR mode, and float16 input. + LOGS(logger, VERBOSE) << "DepthToSpace: DCR mode with float16 input requires at least CoreML version 7."; + return false; + } + } } else { if (mode != "DCR") { LOGS(logger, VERBOSE) << "DepthToSpace: " << mode << " mode is not supported"; diff --git a/onnxruntime/test/logging_apis/test_logging_apis.cc b/onnxruntime/test/logging_apis/test_logging_apis.cc index d72c47493d..b98e5c34b4 100644 --- a/onnxruntime/test/logging_apis/test_logging_apis.cc +++ b/onnxruntime/test/logging_apis/test_logging_apis.cc @@ -359,12 +359,16 @@ TEST_F(MockCAPITestsFixture, CppLogMacroBypassCApiCall) { #undef TEST_MAIN #define TEST_MAIN main_no_link_ // there is a UI test app for iOS. -// IOS tests require this function to be defined. +// iOS tests require ortenv_setup() and ortenv_teardown() to be defined. // See onnxruntime/test/xctest/xcgtest.mm -void ortenv_setup() { +extern "C" void ortenv_setup() { // Do nothing. These logging tests do not require an env to be setup initially. } +extern "C" void ortenv_teardown() { + // Do nothing. +} + #endif // TARGET_OS_SIMULATOR || TARGET_OS_IOS #endif // defined(__APPLE__) diff --git a/onnxruntime/test/unittest_main/test_main.cc b/onnxruntime/test/unittest_main/test_main.cc index 1d89272680..b558a7f00f 100644 --- a/onnxruntime/test/unittest_main/test_main.cc +++ b/onnxruntime/test/unittest_main/test_main.cc @@ -27,8 +27,8 @@ std::unique_ptr ort_env; -// ortenv_setup is used by /onnxruntime/test/xctest/xcgtest.mm so can't be file local -void ortenv_setup() { +// ortenv_setup() and ortenv_teardown() are used by onnxruntime/test/xctest/xcgtest.mm so can't be file local +extern "C" void ortenv_setup() { OrtThreadingOptions tpo; // allow verbose logging to be enabled by setting this environment variable to a numeric log level @@ -46,6 +46,10 @@ void ortenv_setup() { ort_env.reset(new Ort::Env(&tpo, log_level, "Default")); } +extern "C" void ortenv_teardown() { + ort_env.reset(); +} + #ifdef USE_TENSORRT #if defined(_MSC_VER) @@ -101,7 +105,7 @@ int TEST_MAIN(int argc, char** argv) { } // TODO: Fix the C API issue - ort_env.reset(); // If we don't do this, it will crash + ortenv_teardown(); // If we don't do this, it will crash #ifndef USE_ONNXRUNTIME_DLL // make memory leak checker happy diff --git a/onnxruntime/test/xctest/xcgtest.mm b/onnxruntime/test/xctest/xcgtest.mm index c02f18d906..785c9cd937 100644 --- a/onnxruntime/test/xctest/xcgtest.mm +++ b/onnxruntime/test/xctest/xcgtest.mm @@ -34,7 +34,8 @@ using testing::TestInfo; using testing::TestPartResult; using testing::UnitTest; -void ortenv_setup(); +extern "C" void ortenv_setup(); +extern "C" void ortenv_teardown(); static NSString* const GoogleTestDisabledPrefix = @"DISABLED_"; @@ -63,24 +64,51 @@ class XCTestListener : public testing::EmptyTestEventListener { public: XCTestListener(XCTestCase* testCase) : _testCase(testCase) {} - void OnTestPartResult(const TestPartResult& test_part_result) { + void OnTestPartResult(const TestPartResult& test_part_result) override { if (test_part_result.passed() || test_part_result.skipped()) return; int lineNumber = test_part_result.line_number(); const char* fileName = test_part_result.file_name(); NSString* path = fileName ? [@(fileName) stringByStandardizingPath] : nil; + NSString* summary = @(test_part_result.summary()); NSString* description = @(test_part_result.message()); - [_testCase recordFailureWithDescription:description - inFile:path - atLine:(lineNumber >= 0 ? (NSUInteger)lineNumber : 0) - expected:YES]; + + XCTSourceCodeLocation* sourceCodeLocation = + [[XCTSourceCodeLocation alloc] initWithFilePath:path + lineNumber:lineNumber]; + + XCTSourceCodeContext* sourceCodeContext = + [[XCTSourceCodeContext alloc] initWithLocation:sourceCodeLocation]; + + XCTIssue* issue = [[XCTIssue alloc] initWithType:XCTIssueTypeAssertionFailure + compactDescription:summary + detailedDescription:description + sourceCodeContext:sourceCodeContext + associatedError:nil + attachments:@[]]; + + [_testCase recordIssue:issue]; } private: XCTestCase* _testCase; }; +/** + * A Google Test listener that manages the ORT env setup and teardown. + */ +class OrtEnvManagementListener : public testing::EmptyTestEventListener { + public: + void OnTestProgramStart(const UnitTest& unit_test) override { + ortenv_setup(); + } + + void OnTestProgramEnd(const UnitTest& unit_test) override { + ortenv_teardown(); + } +}; + /** * Registers an XCTestCase subclass for each Google Test case. * @@ -179,7 +207,6 @@ static void RunTest(id self, SEL _cmd) { object:bundle queue:nil usingBlock:^(NSNotification* notification) { - ortenv_setup(); [self registerTestClasses]; }]; } @@ -201,6 +228,8 @@ static void RunTest(id self, SEL _cmd) { delete listeners.Release(listeners.default_result_printer()); free(argv); + listeners.Append(new OrtEnvManagementListener()); + BOOL runDisabledTests = GTEST_FLAG_GET(also_run_disabled_tests); NSMutableDictionary* testFilterMap = [NSMutableDictionary dictionary]; NSCharacterSet* decimalDigitCharacterSet = [NSCharacterSet decimalDigitCharacterSet]; diff --git a/tools/ci_build/github/apple/get_simulator_device_info.py b/tools/ci_build/github/apple/get_simulator_device_info.py index 7de9aa1391..aa693038b4 100755 --- a/tools/ci_build/github/apple/get_simulator_device_info.py +++ b/tools/ci_build/github/apple/get_simulator_device_info.py @@ -8,6 +8,7 @@ import argparse import functools import itertools import json +import os import subprocess @@ -37,7 +38,7 @@ class Version: def get_simulator_device_info( requested_runtime_platform: str = "iOS", requested_device_type_product_family: str = "iPhone", - max_runtime_version_str: str | None = None, + requested_runtime_version_str: str | None = None, ) -> dict[str, str]: """ Retrieves simulator device information from Xcode. @@ -45,11 +46,13 @@ def get_simulator_device_info( :param requested_runtime_platform: The runtime platform to select. :param requested_device_type_product_family: The device type product family to select. - :param max_runtime_version_str: The maximum runtime version to allow. + :param requested_runtime_version_str: The runtime version to select. If unspecified, selects the latest one. :return: A dictionary containing information about the selected simulator device. """ - max_runtime_version = Version(max_runtime_version_str) if max_runtime_version_str is not None else None + requested_runtime_version = ( + Version(requested_runtime_version_str) if requested_runtime_version_str is not None else None + ) simctl_proc = subprocess.run( ["xcrun", "simctl", "list", "--json", "--no-escape-slashes"], @@ -73,7 +76,7 @@ def get_simulator_device_info( if runtime["platform"] != requested_runtime_platform: return False - if max_runtime_version is not None and Version(runtime["version"]) > max_runtime_version: + if requested_runtime_version is not None and Version(runtime["version"]) != requested_runtime_version: return False return True @@ -108,6 +111,9 @@ def get_simulator_device_info( ): runtime_id_and_device_pairs.extend((runtime_id, device) for device in filter(device_filter, device_list)) + if len(runtime_id_and_device_pairs) == 0: + raise ValueError("Failed to find requested simulator device info.") + # sort key - tuple of (runtime version, device type min runtime version) # the secondary device type min runtime version value is to treat more recent device types as greater def runtime_id_and_device_pair_key(runtime_id_and_device_pair): @@ -137,13 +143,20 @@ def get_simulator_device_info( def main(): + requested_runtime_version_environment_variable_name = "ORT_GET_SIMULATOR_DEVICE_INFO_REQUESTED_RUNTIME_VERSION" + parser = argparse.ArgumentParser(description="Gets simulator info from Xcode and prints it in JSON format.") - parser.add_argument("--max-runtime-version", help="The maximum runtime version to allow.") + parser.add_argument( + "--requested-runtime-version", + default=os.environ.get(requested_runtime_version_environment_variable_name, None), + help="The requested runtime version. " + f"This may also be specified with the {requested_runtime_version_environment_variable_name} " + "environment variable. The command line option takes precedence. " + "An unspecified value means the latest available runtime version.", + ) args = parser.parse_args() - info = get_simulator_device_info( - max_runtime_version_str=args.max_runtime_version, - ) + info = get_simulator_device_info(requested_runtime_version_str=args.requested_runtime_version) print(json.dumps(info, indent=2)) diff --git a/tools/ci_build/github/azure-pipelines/mac-ios-ci-pipeline.yml b/tools/ci_build/github/azure-pipelines/mac-ios-ci-pipeline.yml index c61beb63b8..9576aac182 100644 --- a/tools/ci_build/github/azure-pipelines/mac-ios-ci-pipeline.yml +++ b/tools/ci_build/github/azure-pipelines/mac-ios-ci-pipeline.yml @@ -36,9 +36,16 @@ jobs: PROTO_CACHE_DIR: $(Pipeline.Workspace)/proto_ccache ORT_CACHE_DIR: $(Pipeline.Workspace)/ort_ccache TODAY: $[format('{0:dd}{0:MM}{0:yyyy}', pipeline.startTime)] + # Note: Keep the Xcode version and iOS simulator version compatible. + # Check the table here to see what iOS simulator versions are supported by a particular Xcode version: + # https://developer.apple.com/support/xcode/ + XCODE_VERSION: 14.3.1 + IOS_SIMULATOR_RUNTIME_VERSION: 16.4 timeoutInMinutes: 150 steps: - template: templates/use-xcode-version.yml + parameters: + xcodeVersion: $(XCODE_VERSION) - template: templates/mac-build-step-with-cache.yml parameters: @@ -71,3 +78,4 @@ jobs: CCACHE_DEPEND: 1 CCACHE_SLOPPINESS: modules CCACHE_DIR: $(ORT_CACHE_DIR) + ORT_GET_SIMULATOR_DEVICE_INFO_REQUESTED_RUNTIME_VERSION: $(IOS_SIMULATOR_RUNTIME_VERSION) diff --git a/tools/ci_build/github/azure-pipelines/templates/stages/mac-ios-packaging-build-stage.yml b/tools/ci_build/github/azure-pipelines/templates/stages/mac-ios-packaging-build-stage.yml index e27de27036..0d23304892 100644 --- a/tools/ci_build/github/azure-pipelines/templates/stages/mac-ios-packaging-build-stage.yml +++ b/tools/ci_build/github/azure-pipelines/templates/stages/mac-ios-packaging-build-stage.yml @@ -18,7 +18,12 @@ stages: vmImage: "macOS-13" variables: + # Note: Keep the Xcode version and iOS simulator version compatible. + # Check the table here to see what iOS simulator versions are supported by a particular Xcode version: + # https://developer.apple.com/support/xcode/ xcodeVersion: "14.3.1" + iosSimulatorRuntimeVersion: "16.4" + ortPodVersion: $[stageDependencies.IosPackaging_SetCommonVariables.j.outputs['SetCommonVariables.ORT_POD_VERSION']] ${{ if eq(parameters.packageVariant, 'Full') }}: @@ -62,6 +67,8 @@ stages: architecture: "x64" - template: ../use-xcode-version.yml + parameters: + xcodeVersion: $(xcodeVersion) - template: ../install-appcenter.yml @@ -80,6 +87,8 @@ stages: --build-settings-file "${{ variables.buildSettingsFile }}" \ ${{ variables.optionalIncludeOpsByConfigOption }} displayName: "Build macOS/iOS framework and assemble pod package files" + env: + ORT_GET_SIMULATOR_DEVICE_INFO_REQUESTED_RUNTIME_VERSION: $(iosSimulatorRuntimeVersion) - script: | python tools/ci_build/github/apple/test_apple_packages.py \