Conversation
Fix org policy violation: pin checkout, setup-python, upload-artifact. Also remove stale requirements-dev.txt reference and trim Python matrix.
This workflow tested SEDA packer tooling from the pre-extraction repo. The tests/ dir has only conftest.py with no actual test files. Native .NET validation is covered by build-rocm-linux.yml.
These files were from SEDA tooling in the pre-extraction repo. The native-only package has no Python dependencies. Removed: main.py, test_vault.py, pyproject.toml, pytest.ini, .coveragerc, rocm_integration.commit.seda, tests/
* fix(ci): set LD_LIBRARY_PATH for ORT native lib loading and accept GetProvider ROCm EP symbol * fix(ci): inject ROCm so into test output dir as onnxruntime.dll to satisfy ORT 1.24.x DllImport * fix(tests): downgrade ORT NuGet to 1.19.2 to match native build, fix ROCm EP catch, drop providers_rocm.so from test output dir * chore: untrack lscache files, add to .gitignore * add sln and VS Code workspace * add MIGraphX EP migration plan from ROCm EP Six-phase plan covering build system, tests, docs, metadata, validation, and release for migrating from deprecated ROCm EP to MIGraphX EP targeting ORT v1.23.2 + ROCm 7.2.1. * add multi-version branching strategy for ORT+ROCm couplets Documents per-version release branches, SemVer scheme mapping ORT+ROCm versions to package versions, and feasibility annotations for tracking multiple version combinations in parallel. * feat: add initial workspace configuration for inference engine * feat: update MIGraphX migration plan to reflect ORT version 1.24.1 and fix ABI mismatch * chore: add whitespace for improved readability in migration and implementation plans * migrate build system from ROCm EP to MIGraphX EP (ORT v1.24.1, ROCm 7.2.1) - Update ORT_TAG to v1.24.1, replace --use_rocm with --use_migraphx - Update Eigen to commit 1d8b82b0 from eigen-mirror (ORT v1.24.1 pin) - Add migraphx-dev to build dependencies, add gfx1101/gfx1102 architectures - Update Docker image to rocm/dev-ubuntu-22.04:7.2.1 - Output libonnxruntime_providers_migraphx.so instead of providers_rocm.so * update integration tests and package metadata for MIGraphX EP - Update ORT NuGet to 1.24.1 to match native build - Rename test methods from Rocm to MIGraphX, check providers_migraphx.so - Use MIGraphXExecutionProvider API in EP registration test - Update package description and tags to mention MIGraphX * update documentation for MIGraphX migration with API change guide - Add migration section to README with ROCm→MIGraphX API change and version table - Update ARCHITECTURE.md build details, consumer examples, and version references - Update AGENTS.md Docker image, artifact names, and MIGraphX EP mention * update migration plan with implementation status and commit references * address review feedback: ship providers_shared.so, fix doc rendering, harden compile script - Ship libonnxruntime_providers_shared.so alongside MIGraphX provider - Add ROCm version component validation (fail fast vs 60-min wasted build) - Handle cached Eigen directory with stale GitLab remote - Fix nested markdown code block rendering in migration plan - Update stale ORT v1.19.2/ROCm 6.0.2 references in plan docs - Add integration test coverage for providers_shared.so * reorder ROCm version validation before version string construction Move the fail-fast validation check before constructing and printing ROCM_VERSION_STRING so the script exits cleanly without printing a misleading malformed version string. * only fetch Eigen from network when commit not available locally Avoids unnecessary git fetch in offline/cached environments by checking if the target commit exists in the local object store before fetching. * Update MIGraphX migration plan for ORT 1.24.1 and improve readability (#2) * feat: update MIGraphX migration plan to reflect ORT version 1.24.1 and fix ABI mismatch * chore: add whitespace for improved readability in migration and implementation plans * migrate build system from ROCm EP to MIGraphX EP (ORT v1.24.1, ROCm 7.2.1) - Update ORT_TAG to v1.24.1, replace --use_rocm with --use_migraphx - Update Eigen to commit 1d8b82b0 from eigen-mirror (ORT v1.24.1 pin) - Add migraphx-dev to build dependencies, add gfx1101/gfx1102 architectures - Update Docker image to rocm/dev-ubuntu-22.04:7.2.1 - Output libonnxruntime_providers_migraphx.so instead of providers_rocm.so * update integration tests and package metadata for MIGraphX EP - Update ORT NuGet to 1.24.1 to match native build - Rename test methods from Rocm to MIGraphX, check providers_migraphx.so - Use MIGraphXExecutionProvider API in EP registration test - Update package description and tags to mention MIGraphX * update documentation for MIGraphX migration with API change guide - Add migration section to README with ROCm→MIGraphX API change and version table - Update ARCHITECTURE.md build details, consumer examples, and version references - Update AGENTS.md Docker image, artifact names, and MIGraphX EP mention * update migration plan with implementation status and commit references * address review feedback: ship providers_shared.so, fix doc rendering, harden compile script - Ship libonnxruntime_providers_shared.so alongside MIGraphX provider - Add ROCm version component validation (fail fast vs 60-min wasted build) - Handle cached Eigen directory with stale GitLab remote - Fix nested markdown code block rendering in migration plan - Update stale ORT v1.19.2/ROCm 6.0.2 references in plan docs - Add integration test coverage for providers_shared.so * reorder ROCm version validation before version string construction Move the fail-fast validation check before constructing and printing ROCM_VERSION_STRING so the script exits cleanly without printing a misleading malformed version string. * only fetch Eigen from network when commit not available locally Avoids unnecessary git fetch in offline/cached environments by checking if the target commit exists in the local object store before fetching. * fix: lower size threshold for provider libs in validation test The MIGraphX provider .so is a thin plugin wrapper (~550 KB) and doesn't meet the 1 MB threshold designed for the main runtime library. Use 100 KB minimum for provider libs, keep 1 MB for libonnxruntime.so. * fix: lower min size threshold for libonnxruntime_providers_shared.so When building ORT with --use_migraphx, the provider shared library is a thin shim (~15KB) that delegates to the MIGraphX-specific .so. The previous 100KB threshold was incorrectly failing this legitimate build artifact. Lower the threshold to 10KB to match the actual output. * fix: address PR #1 review comments - Remove duplicate *.lscache entry from .gitignore (already at line 12) - Replace generic AppendExecutionProvider with typed AppendExecutionProvider_MIGraphX(0) to exercise the native provider loading path, remove NotSupportedException catch that could mask a skipped native load - Add top-level permissions: contents: read to workflow to satisfy CodeQL alerts * docs: fix migration guide — MIGraphX typed API exists in ORT 1.24.1 The guide incorrectly stated MIGraphX has no typed API. AppendExecutionProvider_MIGraphX(int) is available in ORT 1.24.1. --------- Co-authored-by: Nathan Miller <nmiller217@gmail.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request migrates the build system and integration tests from the deprecated ROCm Execution Provider to the recommended MIGraphX Execution Provider, updating ONNX Runtime to v1.24.1 and ROCm to 7.2.1. It updates the compilation script, adapts native library validation tests, adds comprehensive migration plans and guides, and removes obsolete Python tooling. Feedback on these changes suggests making the Eigen source directory validation in the build script more robust against non-git directories, and renaming the 'BothLibs_AreNonEmpty' test method to accurately reflect that it now validates three libraries.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
- Handle leftover non-git Eigen directories in build script - Rename BothLibs_AreNonEmpty to RequiredLibs_AreNonEmpty for accuracy
Member
Author
Review Comments AddressedAll review comments have been resolved in commit b647c62:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.