Conversation
| oss << "Running tests in parallel: at most " | ||
| << static_cast<unsigned>(parallel_models) | ||
| << " models at any time"; | ||
| TEST_LOG_VERBOSE(oss.str()); |
There was a problem hiding this comment.
I think this is a code duplicate and data copy, since the macro itself makes use of the std::ostringstream. Would it be possible to output to macro directly?
There was a problem hiding this comment.
good catch. Used macro directly.
onnxruntime/test/onnx/utils/utils.cc
Outdated
| for (auto& pair : ep_names_to_libs) { | ||
| const std::filesystem::path library_path = pair.second; | ||
| const std::string registration_name = pair.first; | ||
| Ort::Status status(Ort::GetApi().RegisterExecutionProviderLibrary(env, registration_name.c_str(), ToPathString(library_path.string()).c_str())); |
There was a problem hiding this comment.
We should use C++ interfaces.
There was a problem hiding this comment.
changed to use C++ API
onnxruntime/test/onnx/utils/utils.cc
Outdated
| for (auto& registration_name : test_config.registered_plugin_eps) { | ||
| void UnregisterExecutionProviderLibrary(Ort::Env& env, std::vector<std::string>& registered_plugin_eps) { | ||
| for (auto& registration_name : registered_plugin_eps) { | ||
| Ort::Status status(Ort::GetApi().UnregisterExecutionProviderLibrary(env, registration_name.c_str())); |
There was a problem hiding this comment.
changed to use C++ API
|
|
||
| target_link_libraries(onnx_test_runner PRIVATE onnx_test_runner_common ${GETOPT_LIB_WIDE} ${onnx_test_libs} nlohmann_json::nlohmann_json) | ||
| if (onnxruntime_BUILD_SHARED_LIB) | ||
| # onnx_test_runner calls functions from onnxruntime_test_utils, which depend on ORT internal C++ APIs not exposed through the public C API. |
There was a problem hiding this comment.
We need clarufy the purpose and the scope of the PR.
This links the test against many internal libraries. I do not think this is needed or intended as it was not present before.
Also, I thought the purpose of the PR was to link against the DLL, meaning talk to ORT only via public interfaces.
The functionality of the utility is such that it only needs to create sessions and run them in essense.
Very much like onnxruntime_shared_lib_test which is specifically written to test and cover public API.
There was a problem hiding this comment.
Unlike onnxruntime_shared_lib_test, onnx_test_runner needs to compare the output and that functions eventually call ORT internal library that are not exposed as C API.
As the comment described below, it calls CompareOrtValue(), VerifyValueInfo() ... which rely on onnxruntime::DataTypeImpl::TypeFromProto(), onnxruntime::DataTypeImpl::ToString() ... in the internal library, i.e. onnxruntime_framework, to do the output comparison.
To work with plugin EP, onnx_test_runner needs to be link against onnxruntime.dll.
For onnx_test_runner to work with onnxruntime.dll, we need to link the ORT internal library.
(additional internal dependencies such as onnxruntime_graph and onnxruntime_mlas must also be linked,
as they are transitively required by symbols used in onnxruntime_framework.)
There was a problem hiding this comment.
Even though we make onnx_test_runner link many internal libraries, what it really needs is some functions to compare the OrtValues which are all stateless.
There was a problem hiding this comment.
I donot think the comparision logic warrants linking the whole world.
It can be re-written by means public interface. It is not that much code that needs to be changed.
There was a problem hiding this comment.
okay, i though about making the parts needed by comparison logic be public. Will take a look to see how much change it needs.
There was a problem hiding this comment.
There are more changes than i expected.
- Use Ort::Value instead of OrtValue.Type(), OrtValue.Get() ....
- ONNXType - Using Ort::GetApi().GetValueType() to get the type.
- The ONNXType that are supported:
- Tensor
- Sparse Tensor - including COO and Csr(c) format.
- Sequence of Tensors
- Sequence of Maps (where map is either 'int64 to float' or 'string to float')
- (Need to rewrite the functions to compare two OrtValues with all of the above type)
- ORT doesn't have public API to deal with Int4x2 and UInt4x2 tensor type
- MLFloat16 - It's only used in training code, maybe we can remove it?
|
there's build errors and merge conflict |
|
Have a separate PR that add a new |
Description
Make
onnx_test_runnerbe able to link against onnxruntime.dll.Add support for
onnx_test_runnerto register plugin EP DLL and run plugin EP.When running
onnx_test_runnerto test plugin EP, we highly suggest that theonnx_test_runneris built/linked against onnxruntime.dll, so that bothonnx_test_runnerand plugin EP are interacting with the same onnxruntime.dll.[Note]: On Windows, it's possible that incompatible onnxruntime.dll in system folder is used. Make sure correct onnxruntime.dll is either in
onnx_test_runner's folder or plugin EP DLL's folder. ORT callsLoadLibraryExWto load plugin EP DLL with the flagLOAD_WITH_ALTERED_SEARCH_PATH( Windows uses alternative search order so that the directory containing the loaded DLL is searched for that DLL’s dependencies).New options:
--plugin_ep_libs [registration names and libraries]Specifies a list of plugin execution provider (EP) registration names and their corresponding shared libraries to register.[Usage]:
--plugin_ep_libs "plugin_ep_name_1|plugin_ep_1.dll plugin_ep_name_2|plugin_ep_2.dll ... "--plugin_eps [Plugin EPs]Specifies a semicolon-separated list of plugin execution providers (EPs) to use.[Usage]:
--plugin_eps "plugin_ep_1;plugin_ep_2;... "--plugin_ep_options [EP options]Specifies provider options for each EP listed in --plugin_eps. Options (key-value pairs) for each EP are separated by space and EPs are separated by semicolons.[Usage]:
--plugin_ep_options "ep_1_option_1_key|ep_1_option_1_value ...;ep_2_option_1_key|ep_2_option_1_value ...;..."or--plugin_ep_options ";ep_2_option_1_key|ep_2_option_1_value ...;..."or--plugin_ep_options "ep_1_option_1_key|ep_1_option_1_value ...;;ep_3_option_1_key|ep_3_option_1_value ...;..."--list_ep_devicesPrints all available device indices and their properties (including metadata). This option makes the program exit early without performing inference.--select_ep_devices [list of device indices]A semicolon-separated list of device indices to add to the session and run with.Motivation and Context