Skip to content

Add Execution Provider conformance test suite - #28968

Open
GopalakrishnanN wants to merge 8 commits into
mainfrom
GopalakrishnanN/ep-conformance-tests
Open

Add Execution Provider conformance test suite#28968
GopalakrishnanN wants to merge 8 commits into
mainfrom
GopalakrishnanN/ep-conformance-tests

Conversation

@GopalakrishnanN

@GopalakrishnanN GopalakrishnanN commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Description

Adds shared, executable conformance checks for both ONNX Runtime execution-provider integration paths:

  1. Built-in/in-tree EPs through the internal IExecutionProvider interface.
  2. Out-of-tree plugin EPs through the public OrtEp / OrtEpFactory ABI, reached end-to-end via PluginExecutionProvider after dynamically loading the plugin library.

The backend-agnostic invariants live in one header and are reused by both suites so their expectations cannot drift apart.

Files

  • onnxruntime/test/util/include/ep_conformance_invariants.h
    • Single source of truth for the shared invariant checks.
  • onnxruntime/test/framework/execution_provider_conformance_test.cc
    • Parameterized built-in EP suite (EpContract/EpConformanceTest).
  • onnxruntime/test/providers/ep_conformance_plugin_test.cc
    • Dynamic plugin suite (EpPluginConformanceTest) compiled into onnxruntime_provider_test.

Coverage

The built-in suite always covers the CPU EP in arena and non-arena configurations. CUDA, DML, WebGPU, and XNNPACK are registered behind their build guards. A factory that returns nullptr (for example, an EP compiled but unavailable on the current machine) skips the affected test instead of failing.

The plugin suite runs against whichever plugin initializes the existing dynamic-plugin test infrastructure. This includes:

  • in-repo example plugin EPs;
  • ORT EPs routed through the plugin API adapters, such as CUDA or WebGPU;
  • external plugin EPs selected with ORT_UNIT_TEST_MAIN_DYNAMIC_PLUGIN_EP_CONFIG_JSON or ORT_UNIT_TEST_MAIN_DYNAMIC_PLUGIN_EP_CONFIG_JSON_FILE.

If no plugin is configured, the plugin tests skip cleanly.

Invariants enforced

  1. Type() is non-empty and stable across calls and independent instances.
  2. GetPreferredLayout() returns a defined layout (NCHW or NHWC).
  3. CPU input/output memory types map to CPU-accessible devices.
  4. CreatePreferredAllocators() returns no null entries and is repeatable.
  5. CPU-accessible preferred allocators return usable, writable memory that can be freed.
  6. An advertised CPU-to-CPU IDataTransfer round trip preserves bytes exactly.
  7. Metadata queries are callable on a fresh EP and GetDeviceId() agrees with GetDevice().Id().
  8. GetGraphCaptureNodeAssignmentPolicy() returns a defined policy value.
  9. GetOrtEp() matches provider kind: built-in EPs return nullptr; plugin EPs return a stable, non-null backing OrtEp.
  10. GetEpContextNodes() is empty before any compilation has occurred.
  11. Preferred allocators report well-formed OrtMemoryInfo (non-empty name and valid allocator type).

Only documented, backend-agnostic behavior is asserted. Non-CPU-accessible memory is never dereferenced by the test thread; backend-specific operations are skipped when their portable preconditions are unavailable.

Why this change matters

ONNX Runtime relies on more than 20 execution providers being substitutable behind a common framework contract, while external EP authors are expected to implement the public plugin ABI. Previously, the assumptions shared by those paths were implicit: distributed across interface comments, adapter logic, and downstream framework code.

That creates three recurring costs:

  • Contract drift: an EP can return invalid layout/device/allocator metadata and fail later in an unrelated framework path.
  • External authoring cost: plugin authors must reverse-engineer the behavior ORT expects beyond function signatures.
  • Regression risk: changes to allocators, device mapping, data transfer, or the plugin adapter can silently violate a shared assumption.

These suites turn the common baseline into an executable checklist and exercise the plugin ABI through the same adapter path used in production.

Running against an external plugin EP

Build onnxruntime_provider_test, point the dynamic-plugin test infrastructure at the plugin library, and run only the plugin conformance suite. For example:

export ORT_UNIT_TEST_MAIN_DYNAMIC_PLUGIN_EP_CONFIG_JSON='{
  "ep_library_registration_name": "ExampleExecutionProvider",
  "ep_library_path": "/absolute/path/to/libexample_ep.dylib",
  "selected_ep_name": "ExampleExecutionProvider"
}'

./onnxruntime_provider_test --gtest_filter='EpPluginConformanceTest.*'

The registration and selected EP names must match the names exposed by the plugin factory.

Testing

Local Windows RelWithDebInfo validation:

[==========] 22 tests from 1 test suite ran.
[  PASSED  ] 22 tests.

This is 11 shared invariants across the CPU arena and non-arena configurations. Plugin-enabled CI jobs run the same invariant set through the dynamically loaded plugin path; unsupported backend-specific portions skip rather than fail.

External plugin validation on macOS 14 / Apple Silicon:

  • Plugin: onnxruntime-ep-mlx==0.3.0 (MLXExecutionProvider, public plugin ABI version 27).
  • Test binary: onnxruntime_provider_test built from this PR (ORT API version 28).
  • Result: ad hoc MLX validation run completed successfully.
[==========] 11 tests from 1 test suite ran. (37 ms total)
[  PASSED  ] 9 tests.
[  SKIPPED ] 2 tests.

The two skips are expected and contract-valid: MLX exposes no preferred allocator to exercise directly and no IDataTransfer. All applicable shared invariants passed, including plugin identity/lifetime (GetOrtEp()), layout/device metadata, graph-capture policy, and fresh-EP state.

Relationship to EP capability segregation (#29087)

This PR validates the baseline behavior common to every EP. It intentionally does not infer support for optional capabilities from legacy default virtual methods.

#29087 adds explicit capability-query hooks. If that work lands, capability-specific conformance checks (graph capture, tuning, compilation, and data-layout decisions) can be added only for EPs that advertise the corresponding capability. The PRs remain independent: this suite does not require #29087, and #29087 does not require this suite.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds a parameterized GoogleTest suite (EpContract/EpConformanceTest) under onnxruntime/test/framework/ to codify and enforce baseline invariants expected of all IExecutionProvider implementations (e.g., stable Type(), valid preferred layout, CPU memtype mapping, allocator behavior, optional data transfer correctness, and basic metadata query consistency).

Changes:

  • Introduces a new parameterized conformance test fixture for IExecutionProvider contract invariants.
  • Registers CPU EP instances (arena + non-arena) and conditionally registers additional EPs behind USE_* guards.
  • Validates allocator usability and (when applicable) CPU↔CPU IDataTransfer::CopyTensor round-trip correctness.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread onnxruntime/test/framework/execution_provider_conformance_test.cc Outdated
GopalakrishnanN pushed a commit that referenced this pull request Jun 10, 2026
PreferredAllocatorsAllocateUsableMemory previously called Alloc()/Free() on every preferred allocator, only gating the host memory read/write behind UsesCpuMemory(). On WebGPU (arm64 Debug CI) the GpuBufferAllocator creates buffers mapped at creation; Free() routes through the buffer manager which throws EnforceBufferUnmapped ('Buffer is still mapped'), failing the test.

Restrict the standalone Alloc/Free exercise to CPU-accessible allocators, whose lifecycle is backend-agnostic, and GTEST_SKIP when an EP exposes no such allocator. Device allocators remain covered by PreferredAllocatorsAreNonNullAndRepeatable. Fixes the webgpu build-and-test (arm64, Debug) leg on PR #28968.
@edgchen1

Copy link
Copy Markdown
Contributor

it's a good idea to have more tests that EP authors can use for verification.

IExecutionProvider is an internal ORT interface. the preferred way to develop a new EP is as a plugin EP. given that, I think we should treat the plugin EP API (e.g., OrtEp) as the contract and test against that. we should enable these tests for arbitrary plugin EPs too. onnxruntime_provider_test is meant to support plugin EPs and might be a useful reference (it was added in #25689).

@justinchuby

Copy link
Copy Markdown
Contributor

I like the idea of defining invariants clearly

@GopalakrishnanN

Copy link
Copy Markdown
Contributor Author

Thanks @edgchen1 — agreed, the plugin OrtEp ABI is the contract that matters most for external EP authors, and conformance there is more valuable than against the internal IExecutionProvider.

A few thoughts on getting there:

  • The invariants in this suite map fairly cleanly onto the OrtEp surface — GetPreferredLayout()OrtEp::GetPreferredDataLayout (OrtEpDataLayout), CreatePreferredAllocators()OrtEp::CreateAllocator / OrtEpFactory::CreateAllocator, and the data-transfer round-trip → OrtEpFactory::CreateDataTransfer + OrtDataTransferImpl::CanCopy/CopyTensors. So a plugin-level suite is quite feasible; it's mostly re-expressing the same checks against the ABI rather than inventing new invariants.
  • That said, it's a non-trivial chunk of work (driving an EP through OrtEpFactory, mapping OrtMemoryDevice/OrtMemoryInfo, etc.), and onnxruntime_provider_test (Move provider tests to onnxruntime_provider_test and enable use of plugin EPs #25689, with ORT_UNIT_TEST_ENABLE_DYNAMIC_PLUGIN_EP_USAGE) is the right home — I can parameterize over the example plugin EP plus dynamically-specified ones, the same way this suite parameterizes over the USE_* EPs.
  • The in-tree EPs are still IExecutionProvider implementations, so I'd like to keep these checks as an internal baseline that guards against contract drift when those EPs or the shared framework code (allocators, data transfer, layout, device handling) get refactored.

So my proposal: land this PR as the IExecutionProvider baseline, and do a focused follow-up that adds an OrtEp-level conformance suite in onnxruntime_provider_test reusing these invariant definitions. If you'd rather I hold this PR and pivot straight to the plugin-EP version, I'm happy to do that instead — just let me know which you prefer.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

@edgchen1

Copy link
Copy Markdown
Contributor

So my proposal: land this PR as the IExecutionProvider baseline, and do a focused follow-up that adds an OrtEp-level conformance suite in onnxruntime_provider_test reusing these invariant definitions. If you'd rather I hold this PR and pivot straight to the plugin-EP version, I'm happy to do that instead — just let me know which you prefer.

I suppose it wouldn't hurt to test both the internal interface and the plugin EP interface, but I think the latter would be more useful for EP development going forward. Some of the ORT-owned EPs (like CUDA and WebGPU) have already been converted to plugin EPs.

Gopalakrishnan Nallasamy added 3 commits July 6, 2026 19:33
Parameterized GoogleTest suite (EpContract/EpConformanceTest) encoding universal IExecutionProvider invariants every EP must satisfy: stable Type(), valid preferred layout, CPU mem types map to CPU-accessible devices, non-null/repeatable preferred allocators, usable allocations, CPU data-transfer round-trip integrity, and consistent metadata queries. EPs are registered via factories (no static-init construction); unavailable EPs skip rather than fail. Adding a new EP is a single guarded line in GetEpConformanceParams().
PreferredAllocatorsAllocateUsableMemory previously called Alloc()/Free() on every preferred allocator, only gating the host memory read/write behind UsesCpuMemory(). On WebGPU (arm64 Debug CI) the GpuBufferAllocator creates buffers mapped at creation; Free() routes through the buffer manager which throws EnforceBufferUnmapped ('Buffer is still mapped'), failing the test.

Restrict the standalone Alloc/Free exercise to CPU-accessible allocators, whose lifecycle is backend-agnostic, and GTEST_SKIP when an EP exposes no such allocator. Device allocators remain covered by PreferredAllocatorsAreNonNullAndRepeatable. Fixes the webgpu build-and-test (arm64, Debug) leg on PR #28968.
Address PR review: in ORT_USE_EP_API_ADAPTERS builds, DefaultWebGpuExecutionProvider() ORT_ENFORCEs (aborting the entire unit-test run) when the dynamic plugin EP is initialized to a different EP name, instead of cleanly returning nullptr. Mirror the guard used by base_tester.cc and default_providers.cc by listing the built-in WebGPU EP only under 'defined(USE_WEBGPU) && !defined(ORT_USE_EP_API_ADAPTERS)'.
@GopalakrishnanN
GopalakrishnanN force-pushed the GopalakrishnanN/ep-conformance-tests branch from 9a99a99 to 84c3fd4 Compare July 7, 2026 02:35
…ext nodes, allocator info

Extends the conformance suite with backend-agnostic checks that use only the base IExecutionProvider API (no dependency on the capability-segregation work):

- GraphCaptureNodeAssignmentPolicyIsValid: policy is a defined enum value.

- GetOrtEpIsNullForBuiltInEp: built-in EPs are not plugin-wrapped.

- EpContextNodesEmptyOnFreshEp: no EPContext nodes before any compilation.

- PreferredAllocatorInfoIsConsistent: allocator OrtMemoryInfo has a non-empty name and a valid allocator type.

Also extends MetadataQueriesAreCallable with IsGraphCaptureEnabled() and ShouldConvertDataLayoutForOp() smoke calls. All 22 instances (11 invariants x 2 CPU configs) pass.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comment thread onnxruntime/test/framework/execution_provider_conformance_test.cc Outdated
Gopalakrishnan Nallasamy added 2 commits July 10, 2026 10:20
Extract the backend-agnostic EP conformance invariants into a shared header (test/util/include/ep_conformance_invariants.h) so the built-in and plugin suites share one source of truth.

Add a plugin EP conformance suite in onnxruntime_provider_test (test/providers/ep_conformance_plugin_test.cc) that runs the same invariants against a dynamically-loaded plugin EP -- an OrtEp reached through the PluginExecutionProvider wrapper -- obtained from dynamic_plugin_ep_infra::MakeEp(). It covers arbitrary plugin EPs (example plugin EP, CUDA/WebGPU-as-plugin, or a runtime-specified one) and skips cleanly when no plugin EP is configured. Asserts the plugin-specific stable non-null GetOrtEp().

Refactor the built-in suite (test/framework/execution_provider_conformance_test.cc) to delegate to the shared invariants (behavior-preserving).

Addresses review feedback to also test the plugin OrtEp interface for arbitrary plugin EPs in onnxruntime_provider_test, reusing the invariant definitions.
@GopalakrishnanN

Copy link
Copy Markdown
Contributor Author

@edgchen1 done — the plugin-EP conformance suite is in, so we now cover both interfaces, with the plugin OrtEp path as the emphasis you called out. Pushed in 2ffa0f7.

  • Shared invariants (onnxruntime/test/util/include/ep_conformance_invariants.h): the backend-agnostic invariants are now header-only free functions, so the built-in and plugin suites share one source of truth.
  • Plugin suite (onnxruntime/test/providers/ep_conformance_plugin_test.cc, built into onnxruntime_provider_test): runs those invariants against a dynamically-loaded plugin EP from dynamic_plugin_ep_infra::MakeEp() — an OrtEp exercised through the PluginExecutionProvider wrapper. It is parameterized over whatever plugin EP the infra is initialized with, so it covers the example plugin EP, ORT-owned EPs routed as plugins (CUDA/WebGPU), and any EP specified at runtime via ORT_UNIT_TEST_MAIN_DYNAMIC_PLUGIN_EP_CONFIG_JSON[_FILE]. It skips cleanly when no plugin EP is configured, and adds the plugin-specific invariant that GetOrtEp() is stable and non-null.
  • Built-in baseline (onnxruntime/test/framework/execution_provider_conformance_test.cc) refactored to delegate to the shared invariants (behavior-preserving), retained as an internal contract-drift guard.

The suite drives the plugin through the existing onnxruntime_provider_test infra, i.e. the wrapper forwards each call to the OrtEp/OrtEpFactory ABI — that is what let me reuse the invariant definitions directly. If you would like tighter, raw-ABI coverage for any specific invariant, I can push those down to direct OrtEp/OrtEpFactory calls.

Verified locally (CPU RelWithDebInfo): built-in suite 22/22; plugin suite vs the example plugin EP 10 pass + 1 skip (the example EP advertises no CPU<->CPU data transfer, so that round-trip check skips). The CUDA-plugin and WebGPU-plugin CI legs exercise it against those plugin EPs.

Kept in this PR per your "test both" note — happy to split the plugin suite into its own PR if you would prefer a smaller review.

@justinchuby

Copy link
Copy Markdown
Contributor

I recently created https://pypi.org/project/onnxruntime-ep-mlx. Would be a good candidate for a conformance test.

@GopalakrishnanN

Copy link
Copy Markdown
Contributor Author

Validated the new plugin conformance suite against onnxruntime-ep-mlx==0.3.0 on macOS 14 / Apple Silicon.

All applicable shared invariants passed, including plugin identity/lifetime, layout/device metadata, graph-capture policy, and fresh-EP state. I also added this result and the external-plugin invocation instructions to the PR description.

@justinchuby

Copy link
Copy Markdown
Contributor

Would implementing IDataTransfer be useful?

@GopalakrishnanN

GopalakrishnanN commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

@justinchuby I checked the MLX v0.3.0 implementation more closely. For its current memory model, implementing IDataTransfer solely to remove the conformance skip would not add useful behavior.

MLX intentionally exposes no ORT-visible device memory: OrtEp::GetDefaultMemoryDevice returns null, and the factory's CreateAllocator and CreateDataTransfer callbacks return null. ORT-visible boundary tensors therefore use CPU allocations. During Compute, dynamic ORT inputs are borrowed into MLX with mlx_array_new_data_managed (zero-copy when the buffer meets Metal's alignment requirements, with MLX falling back to a copy otherwise); MLX evaluates synchronously, then copies each materialized result into the ORT-owned CPU output buffer. Because no distinct MLX OrtMemoryDevice/allocator is advertised, ORT's DataTransferManager has no CPU<->MLX device pair for an MLX IDataTransfer to serve.

It would become useful if MLX exposes an ORT-visible MLX memory domain/allocator (even host-accessible unified memory), needs explicit synchronization/copies between device identities, or wants values to remain in MLX memory across EP boundaries. Then it should implement CanCopy/CopyTensors for the actual CPU<->MLX and MLX<->MLX pairs.

One correction to my earlier wording: the current DataTransferCpuRoundTripPreservesData conformance check would not automatically fully validate such a future implementation. It only exercises a same-device round trip on a CPU-accessible preferred allocator; real MLX device-transfer support would need pair-specific host<->MLX tests as well.

@GopalakrishnanN

Copy link
Copy Markdown
Contributor Author

@justinchuby, @edgchen1, is there anything more that needs to be addressed or is it good to get your approval now?

justinchuby
justinchuby previously approved these changes Jul 24, 2026

@justinchuby justinchuby left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM but I would leave it to members more familiar with this component.

@GopalakrishnanN

Copy link
Copy Markdown
Contributor Author

Review summary

Overall this is well-built and well-documented, and it's correctly wired into the build (the test/framework/*.cc, test/providers/*.cc, and test/util/include/*.h globs pick up all three files; the plugin macro is defined for onnxruntime_provider_test, so the plugin suite is not dead code). The invariants are genuinely grounded in the real IExecutionProvider contract — 10 of the 11 verify cleanly against main.

Three issues are worth addressing before merge. They're all the same shape — spurious failures / false coverage confidence, not crashes — so none is structurally blocking.

Major

1. GetDeviceId() == GetDevice().Id() is not a real invariantep_conformance_invariants.h, CheckMetadataQueriesAreCallable
GetDeviceId() is virtual and provider-defined; GetDevice() returns the memory-placement device. WebGPU deliberately decouples them: GetDeviceId() returns the configurable context_id_ (webgpu_execution_provider.h), while its WebGpuDevice id is hardcoded to 0 (webgpu/allocator.h). The check passes today only because the default config uses context_id == 0; a non-default deviceId, or any conformant plugin EP returning a logical device id, would false-fail this "backend-agnostic" invariant.
Suggested fix: drop the EXPECT_EQ and keep it as a smoke call, (void)ep.GetDeviceId(); — there's no documented contract tying the two together.

2. The "factory returns nullptr ⇒ skip" resilience doesn't hold for factories that throwexecution_provider_conformance_test.cc, MakeEp() + the per-test skip pattern
The suite documents that an EP that's compiled-but-unavailable (e.g. no GPU present) is skipped rather than failed. But DefaultCudaExecutionProvider() → CreateProvider() constructs CUDAExecutionProvider, whose constructor calls CUDA_CALL_THROW(cudaSetDevice(...)) / cudaDeviceSynchronize() / cudaGetDeviceProperties() (cuda_execution_provider.cc:341-345). On a USE_CUDA build with no CUDA device/driver, construction throws, so every CUDA conformance test is reported failed, not skipped. Mainline GPU CI is unaffected; the gap hits local/dev runs and misconfigured machines, and it contradicts the suite's stated behavior.
Suggested fix: wrap the factory call in a try/catch and GTEST_SKIP() on a recognized "unavailable" outcome, while preserving genuine init errors as failures.

3. "Every EP" is enforced for only five EPsexecution_provider_conformance_test.cc, GetEpConformanceParams()
Coverage is a manual list: CPU, CUDA, DML, WebGPU, XNNPACK. OpenVINO, TensorRT, QNN, CoreML, CANN, etc. get no parameter, so those EPs can violate every invariant while CI stays green. This is partly by design (adding one is a single guarded line), but nothing forces a new EP to be added.
Suggested fix (or an explicit decision to defer): register conformance factories from the provider registry and assert that every compiled EP is represented — or soften the "every EP" framing in the description to match what's actually enforced.

Minor

  • EXPECT_FALSE(info.name.empty()) (ep_conformance_invariants.h, CheckPreferredAllocatorInfoIsConsistent) over-states the contract — OrtMemoryInfo permits an empty allocator name, so a plugin allocator created via CreateMemoryInfo("", …) is degenerate but arguably conformant and would fail here. The alloc_type != OrtInvalidAllocator half is solid. All in-tree EPs name their allocators, so real-world risk is limited to external plugins — consider relaxing to a warning, or confirm you intend to require a non-empty name.
  • "Repeatable" check is size-only (CheckPreferredAllocatorsAreNonNullAndRepeatable) — it compares .size() of two calls and null-checks only the first call's entries. A second call returning nulls or different allocator identities with the same count would still pass. Consider null-checking the second vector and comparing (name, alloc_type, device) element-wise.
  • "round-trip" naming (CheckDataTransferCpuRoundTripPreservesData) — the body performs a single one-leg CPU→CPU copy; either rename to reflect a one-way copy or perform an actual there-and-back sequence.
  • Skip/fatal-in-helper is a foot-gun (ep_conformance_invariants.h header comment) — GTEST_SKIP() and fatal ASSERT_* inside a Check* helper only return from the helper, not the TEST. All 22 current call sites correctly invoke the helper as the last statement, but a future statement appended after a Check*() call would run even after a skip. Consider returning a structured result and doing skip/fatal control flow in the test body; at minimum, repeat the "call as the last statement" note at the two skip-capable function definitions.
  • expects_plugin_ep is an undocumented bare true (execution_provider_conformance_test.cc, GetEpConformanceParams()) — the CUDA entry passes a trailing literal true, whereas four lines up the same file uses DefaultCpuExecutionProvider(/*enable_arena*/ true). ep_conformance_plugin_test.cc already uses /*expects_plugin_ep*/ true correctly. Either make the struct field a small enum (kBuiltIn/kPlugin) or match the /*expects_plugin_ep=*/true convention, and extend the struct comment to cover the third field.

Nits

  • ep_conformance_plugin_test.cc — the include of test/unittest_util/test_dynamic_plugin_ep.h sits outside the ORT_UNIT_TEST_ENABLE_DYNAMIC_PLUGIN_EP_USAGE guard. Safe today (minimal builds strip providers/*.cc entirely), but cleaner inside the guard.
  • ep_conformance_plugin_test.ccnamespace dynamic_plugin_ep_infra = onnxruntime::test::dynamic_plugin_ep_infra; aliases the namespace to itself; looks like a copy-paste leftover.
  • The "construct a second instance" lambda differs between the two suites ([this] { return MakeEp(); } vs [] { return EpPluginConformanceTest::MakeEp(); }); the simpler form would preserve the otherwise-excellent symmetry.

What's solid

The DataLayout and graph-capture-policy enum checks are provably exhaustive (exactly two live values each on both the built-in and C-API paths); the CUDA plugin-vs-built-in GetOrtEp() expectation mirrors default_providers.cc with no drift; the CPU-mem-type invariant correctly handles CUDA's HOST_ACCESSIBLE override; the WebGPU GpuBufferAllocator Free()-throws exclusion is real and correctly reasoned; and the naming symmetry across the three files makes the built-in suite, plugin suite, and shared header trivially navigable side-by-side.

Note: this is a static, source-grounded review against main — nothing was compiled or executed. The two false-failure findings above are proven from source, not observed at runtime.

- Skip (not fail) when an EP factory throws during construction, e.g. CUDA's
  cudaSetDevice on a GPU-less machine, by wrapping MakeEp() in try/catch and
  logging the exception text before returning nullptr.
- Stop asserting GetDeviceId() == GetDevice().Id(): GetDeviceId() is
  provider-defined (WebGPU returns a configurable context id while its OrtDevice
  id is fixed at 0) and is not required to match; it is now only smoke-called.
- Drop the non-empty allocator-name assertion: an empty OrtMemoryInfo.name is
  permitted by the contract, so requiring one over-states it.
- Strengthen CreatePreferredAllocators() repeatability: null-check the second
  call and compare per-position OrtMemoryInfo, not just the element count.
- Rename DataTransferCpuRoundTrip* to DataTransferCpuCopy* across both suites
  and the shared helper (it is a single CPU-to-CPU copy, not a round-trip).
- Self-document the plugin-EP factory flag (/*expects_plugin_ep=*/true), clarify
  in the header that coverage is opt-in per registered EP, and note that
  skip-capable helpers must be invoked as the last statement of a test body.
- Plugin suite: move test-only includes inside the dynamic-plugin guard, drop a
  redundant self-namespace alias, and simplify a factory lambda.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@GopalakrishnanN

Copy link
Copy Markdown
Contributor Author

Following up on the coverage point from the review above (the suite only covers 5 EPs, and a newly added EP is silently not conformance-tested).

I looked into wiring this to the provider registry rather than the hand-maintained #ifdef USE_* list, and it's tractable: GetAvailableExecutionProviderNames() (core/providers/get_execution_providers.h) already enumerates exactly the EPs compiled into the build, and every Default*ExecutionProvider() is declared unconditionally and returns nullptr when its EP isn't compiled — so the per-EP guards could be dropped and coverage asserted against the registry.

Keeping it out of this PR deliberately: widening the list would start running the invariants against EPs that have never been checked (QNN, OpenVINO, TensorRT, MIGraphX, CANN, CoreML...), which is the point of the change but is better landed as its own ratchet than bundled here.

Tracked in #29914, with the proposed design, the four EPs that would need documented exemptions, and a staging plan.

The allocator-name assertion was removed from the invariant helper, but the
test-side comment still claimed a non-empty name is required.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
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.

4 participants