[AIMIGRAPHX-911] Add callback function to eval() by eddieliao · Pull Request #4780 · ROCm/AMDMIGraphX

eddieliao · 2026-04-13T16:41:23Z

Motivation

Allow users to inspect data from each instruction.

Technical Details

Adds a callback function to eval() that can be used to get data from specific instructions in the compiled graph. See examples/migraphx/cpp_trace_callback for an implementation of a basic callback function.

Changelog Category

Add a CHANGELOG.md entry for any option other than Not Applicable

- Added: New functionality.
- Changed: Changes to existing functionality.
- Removed: Functionality or support that has been removed. (Compared to a previous release)
- Optimized: Component performance that has been optimized or improved.
- Resolved Issues: Known issues from a previous version that have been resolved.
- Not Applicable: This PR is not to be included in the changelog.

Copilot

Pull request overview

Adds a per-instruction evaluation callback to MIGraphX program execution so callers can inspect operator outputs as the graph runs.

Changes:

Introduces migraphx::eval_callback and a new program::eval(..., eval_callback, ...) overload to invoke a callback during evaluation.
Exposes the feature through the C API / C++ API wrapper (migraphx_program_run_callback, program::eval(pparams, F callback)).
Adds unit tests plus a C++ example (examples/migraphx/cpp_eval_callback) demonstrating filtering options.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
`src/program.cpp`	Implements `program::eval` overload and wires callback invocation + host-copy behavior into the eval path.
`src/include/migraphx/program.hpp`	Declares new `eval` overload taking `eval_callback`.
`src/include/migraphx/eval_callback.hpp`	Adds the new callback type with name/instruction filtering helpers.
`src/api/api.cpp`	Adds C API entrypoint `migraphx_program_run_callback` and bridges to `eval_callback`.
`src/api/include/migraphx/migraphx.h`	Declares the C API callback typedef + function.
`src/api/include/migraphx/migraphx.hpp`	Adds a C++ convenience wrapper overload to register a per-instruction callback.
`test/eval_test.cpp`	Adds tests validating callback enablement, firing, and filtering behavior.
`examples/migraphx/cpp_eval_callback/eval_callback_example.cpp`	Provides a runnable example demonstrating callback usage + filtering.
`examples/migraphx/cpp_eval_callback/README.md`	Documents building/running the example and filter modes.
`examples/migraphx/cpp_eval_callback/CMakeLists.txt`	Build configuration for the new example.

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

pfultz2 · 2026-04-13T16:57:44Z

+                                         void* data);
+
+MIGRAPHX_C_EXPORT migraphx_status
+migraphx_program_run_callback(migraphx_arguments_t* out,


This should be called migraphx_program_run_trace to make it clear its enabling the trace.

Or maybe we should not expose another overload and expose the execution_environment.

I've left it exposed through the API for now since I believe it's cleaner than just giving users the execution_environment; I've modified the example as well to show usage of this in trace_callback_example.cpp.

It just starts create more complicated combinations that user might want to do. What if they want to pass a stream and do trace callback? I think for these more advance cases it makes sense to have the user do it through an execution_environment which we they can use the full options for execution.

codecov · 2026-04-13T18:21:14Z

Codecov Report

❌ Patch coverage is 20.31250% with 51 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/program.cpp	27.27%	32 Missing ⚠️
src/api/api.cpp	5.00%	19 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #4780      +/-   ##
===========================================
+ Coverage    92.29%   92.45%   +0.16%     
===========================================
  Files          580      584       +4     
  Lines        28688    29598     +910     
===========================================
+ Hits         26475    27362     +887     
- Misses        2213     2236      +23

Files with missing lines	Coverage Δ
src/api/include/migraphx/migraphx.hpp	`98.79% <ø> (ø)`
src/include/migraphx/execution_environment.hpp	`100.00% <ø> (ø)`
src/api/api.cpp	`70.47% <5.00%> (-1.74%)`	⬇️
src/program.cpp	`72.63% <27.27%> (+0.76%)`	⬆️

... and 35 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.

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

pfultz2 · 2026-04-14T22:46:43Z

    {
-        ret = generic_eval(*this, contexts, params, [&](auto&&, auto f) { return f(); });
+        auto trace_level = value_of(MIGRAPHX_TRACE_EVAL{});
+        if(trace_level > 0)


So I dont think we need two paths for the tracing. Instead when MIGRAPHX_TRACE_EVAL is set, then we set the trace callback and run the same trace code.

Are you suggesting that MIGRAPHX_TRACE_EVAL should overwrite any callback function the user passes in with the current trace callback? Or do we want to support multiple callbacks at once (i.e. both trace and user callback at the same time).

Just overwrite for now.

Co-authored-by: anisha-amd <anisha.sankar@amd.com>

shivadbhavsar

LGTM. You can unify the trace_eval here if that's decided, or the unification can done after in a separate PR too. Re-request if you decide to do it in this pr

pfultz2

Tests needed to be added for the API calls.

eddieliao added 5 commits April 1, 2026 23:32

Claude Opus prototype

7638a00

Add example of callback

bce0448

Add example for eval callback

369cb71

Update examples

a71e18a

Update comment

c26defe

eddieliao self-assigned this Apr 13, 2026

eddieliao added enhancement New feature or request Changelog: Added New functionality. labels Apr 13, 2026

eddieliao requested a review from Copilot April 13, 2026 16:41

Copilot started reviewing on behalf of eddieliao April 13, 2026 16:44 View session

Copilot AI reviewed Apr 13, 2026

View reviewed changes

Comment thread examples/migraphx/cpp_eval_callback/eval_callback_example.cpp Outdated

Comment thread src/include/migraphx/program.hpp Outdated

pfultz2 reviewed Apr 13, 2026

View reviewed changes

Comment thread src/include/migraphx/program.hpp Outdated

pfultz2 reviewed Apr 13, 2026

View reviewed changes

Comment thread src/include/migraphx/eval_callback.hpp Outdated

pfultz2 reviewed Apr 13, 2026

View reviewed changes

Address comments, modify example to use public api

b5088d0

eddieliao requested a review from Copilot April 13, 2026 22:14

Copilot started reviewing on behalf of eddieliao April 13, 2026 22:16 View session

Licensing

6a0d65a

Copilot AI reviewed Apr 13, 2026

View reviewed changes

pfultz2 reviewed Apr 14, 2026

View reviewed changes

Comment thread src/api/api.cpp Outdated

Use op name instead of symbol name

e6665f1

pfultz2 reviewed Apr 14, 2026

View reviewed changes

Comment thread src/include/migraphx/execution_environment.hpp Outdated

pfultz2 reviewed Apr 14, 2026

View reviewed changes

Comment thread src/api/api.cpp Outdated

eddieliao added 5 commits April 15, 2026 18:01

Address comments

d1a23ee

Run make generate

78c7540

Fix API generation

d1881d0

Fix cppcheck and tidy

6f89219

Formatting

87ae690

eddieliao marked this pull request as ready for review April 16, 2026 19:03

eddieliao requested review from a team and causten as code owners April 16, 2026 19:03

eddieliao requested a review from shivadbhavsar April 16, 2026 19:03

Update changelog

93829b4

anisha-amd reviewed Apr 16, 2026

View reviewed changes

Comment thread CHANGELOG.md Outdated

anisha-amd approved these changes Apr 16, 2026

View reviewed changes

Update CHANGELOG.md

a43289d

Co-authored-by: anisha-amd <anisha.sankar@amd.com>

eddieliao requested a review from pfultz2 April 17, 2026 16:33

eddieliao changed the title ~~Add callback function to eval()~~ [AIMIGRAPHX-911] Add callback function to eval() Apr 21, 2026

eddieliao requested a review from ahsan-ca April 21, 2026 20:18

shivadbhavsar approved these changes Apr 22, 2026

View reviewed changes

ahsan-ca approved these changes Apr 22, 2026

View reviewed changes

pfultz2 requested changes Apr 22, 2026

View reviewed changes

Conversation

eddieliao commented Apr 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Technical Details

Changelog Category

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pfultz2 Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

pfultz2 Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

eddieliao Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

pfultz2 Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Apr 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pfultz2 Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

eddieliao Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

pfultz2 Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

shivadbhavsar left a comment

Choose a reason for hiding this comment

Uh oh!

pfultz2 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

eddieliao commented Apr 13, 2026 •

edited

Loading

codecov Bot commented Apr 13, 2026 •

edited

Loading