Skip to content

[onnxruntime/build] Add CI testing for ORT build with generic interface #23530

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.

Sign up for GitHub

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

Jump to bottom
Merged
snnn merged 3 commits into from
Feb 20, 2025
Merged

[onnxruntime/build] Add CI testing for ORT build with generic interface #23530

snnn merged 3 commits into from
Feb 20, 2025

Conversation

@karim-vad
Copy link
Contributor

@karim-vad karim-vad commented Jan 29, 2025 •
edited
Loading

[onnxruntime/build] Add CI testing for ORT build with generic interface

Summary:
- Remove unused cmake variables
- Add target specific logic when generic interface is used.
- Add QNN EP test case that use ORT generic interface build

github-actions[bot]
github-actions bot reviewed Jan 29, 2025
View reviewed changes
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

You can commit the suggested changes from lintrunner.

github-advanced-security[bot]
github-advanced-security bot found potential problems Jan 29, 2025
View reviewed changes
@karim-vad karim-vad force-pushed the dev/kvadsariya/generic_interface_ci_test branch 2 times, most recently from b6e951e to 64c3a26 Compare January 29, 2025 08:12
github-actions[bot]
github-actions bot reviewed Jan 29, 2025
View reviewed changes
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

You can commit the suggested changes from lintrunner.

@karim-vad karim-vad force-pushed the dev/kvadsariya/generic_interface_ci_test branch from fbc3229 to d53cf44 Compare January 30, 2025 21:37
@karim-vad karim-vad changed the base branch from main to win-ort-main January 31, 2025 21:10
@karim-vad karim-vad requested review from a team as code owners January 31, 2025 21:10
@karim-vad karim-vad changed the base branch from win-ort-main to main January 31, 2025 21:13
@adrianlizarraga
Copy link
Contributor

adrianlizarraga commented Feb 11, 2025

Hi @karim-vad, I'm not fully understanding what this PR is testing. Is it essentially running CI/tests with the combinations:

  • --enable_generic_interface --use_qnn
  • --enable_generic_interface --use_tensorrt
  • etc

@karim-vad
Copy link
Contributor Author

karim-vad commented Feb 11, 2025 •
edited
Loading

Hi @karim-vad, I'm not fully understanding what this PR is testing. Is it essentially running CI/tests with the combinations:

  • --enable_generic_interface --use_qnn
  • --enable_generic_interface --use_tensorrt
  • etc

@adrianlizarraga this PR is forcing all the onnxruntime.dll builds with enable_generic_interface flags when the existing CI tests uses one of the primary EPs ( OpenVino, TensorRT, QNN) . The successful execution of the existing CI test validates the Generic interface for ORT is build correctly.

In addition to that, cmake system is incorrectly settings certain pre-processor macros when generic interface is enabled. so this PR fixes those as well.

The next steps is to figure out how we want to enable CI testing with generic interface flag of ORT ?

  1. Create new set of CI tests that validates ORT + EP combos ?
  2. Leverage existing CI tests to set the generic interface flag when OpenVino or TensorRT or QNN EP in use ?

@adrianlizarraga
Copy link
Contributor

adrianlizarraga commented Feb 11, 2025

In addition to that, cmake system is incorrectly settings certain pre-processor macros when generic interface is enabled. so this PR fixes those as well.

I see. If these cmake changes should be merged into main, they should probably be extracted and put into a separate PR. (since we are not on merging this experimental PR). This would allow us to get reviews from others who maintain build configs.

The next steps is to figure out how we want to enable CI testing with generic interface flag of ORT ?

  1. Create new set of CI tests that validates ORT + EP combos ?
  2. Leverage existing CI tests to set the generic interface flag when OpenVino or TensorRT or QNN EP in use ?

From this comment, @snnn indicated that testing all EP combos would not be acceptable due to doubling of resources. Perhaps we can limit this to QNN, TensorRT, OpenVINO. We can start with a PR that only enables this with QNN (as an example). We can use the yaml "strategy matrix" to run tests with --enable_generic_interface --use_qnn:

onnxruntime/tools/ci_build/github/azure-pipelines/win-qnn-arm64-ci-pipeline.yml

Lines 49 to 54 in 989c96c

strategy:
matrix:
SHARED_LIB:
QnnLibKind: 'shared_lib'
STATIC_LIB:
QnnLibKind: 'static_lib' 

  strategy:
    matrix:
      SHARED_LIB:
        QnnLibKind: 'shared_lib'
        qnn_build_args: '--use_qnn'
      STATIC_LIB:
        QnnLibKind: 'static_lib'
        qnn_build_args: '--use_qnn'
      SHARED_LIB_GENERIC_INTERFACE:
        QnnLibKind: 'shared_lib'
        qnn_build_args: '--use_qnn --use_generic_interface'

Maybe something like the above to get us going.

[onnxruntime/build] Add CI testing for ORT build with generic interface
ee5cf8b 
Summary:
- Remove unused cmake variables
- Add target specific logic when generic interface is used.
- Add QNN EP test case that use ORT generic interface build
@karim-vad karim-vad force-pushed the dev/kvadsariya/generic_interface_ci_test branch from d53cf44 to ee5cf8b Compare February 13, 2025 21:58
@karim-vad karim-vad changed the title [DO_NOT_MERGE] For CI validation/Experiment with ORT generic interface [onnxruntime/build] Add CI testing for ORT build with generic interface Feb 13, 2025
@snnn
Copy link
Contributor

snnn commented Feb 20, 2025

/azp run Linux GPU CI Pipeline

@azure-pipelines
Copy link

azure-pipelines bot commented Feb 20, 2025

Azure Pipelines successfully started running 1 pipeline(s).

@snnn snnn merged commit 0babb10 into main Feb 20, 2025
97 of 99 checks passed
@snnn snnn deleted the dev/kvadsariya/generic_interface_ci_test branch February 20, 2025 20:34
guschmue pushed a commit that referenced this pull request Mar 6, 2025
@karim-vad @guschmue
[onnxruntime/build] Add CI testing for ORT build with generic interfa…
20d4d38 
…ce (#23530)

[onnxruntime/build] Add CI testing for ORT build with generic interface

    Summary:
    - Remove unused cmake variables
    - Add target specific logic when generic interface is used.
    - Add QNN EP test case that use ORT generic interface build
ashrit-ms pushed a commit that referenced this pull request Mar 17, 2025
@karim-vad @ashrit-ms
[onnxruntime/build] Add CI testing for ORT build with generic interfa…
f2d3ea4 
…ce (#23530)

[onnxruntime/build] Add CI testing for ORT build with generic interface

    Summary:
    - Remove unused cmake variables
    - Add target specific logic when generic interface is used.
    - Add QNN EP test case that use ORT generic interface build
@karim-vad karim-vad mentioned this pull request Apr 9, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments

@adrianlizarraga adrianlizarraga adrianlizarraga approved these changes

+1 more reviewer

@snnn snnn snnn approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@karim-vad @adrianlizarraga @snnn