AMDMIGraphX icon indicating copy to clipboard operation
AMDMIGraphX copied to clipboard

Introduce Op Builder API

Open mirza-halilcevic opened this issue 1 year ago • 3 comments

  • Implement a separate API that performs the graph construction for specific operators instead of it being done directly in the parser
  • Resolves https://github.com/migraphx-benchmark/AMDMIGraphX/issues/190

mirza-halilcevic avatar Jul 31 '24 12:07 mirza-halilcevic

Codecov Report

:x: Patch coverage is 98.67725% with 5 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/op/builder/einsum.cpp 98.91% 3 Missing :warning:
src/op/builder/op_builder.cpp 80.00% 2 Missing :warning:
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #3328      +/-   ##
===========================================
+ Coverage    92.24%   92.58%   +0.34%     
===========================================
  Files          495      530      +35     
  Lines        19849    26819    +6970     
===========================================
+ Hits         18309    24829    +6520     
- Misses        1540     1990     +450     
Files with missing lines Coverage Δ
src/onnx/parse_einsum.cpp 100.00% <100.00%> (+1.09%) :arrow_up:
src/onnx/parse_gelu.cpp 100.00% <100.00%> (ø)
src/onnx/parse_gemm.cpp 100.00% <100.00%> (ø)
src/op/builder/gelu.cpp 100.00% <100.00%> (ø)
src/op/builder/gemm.cpp 100.00% <100.00%> (ø)
...builder/include/migraphx/op/builder/op_builder.hpp 100.00% <100.00%> (ø)
src/op/builder/op_builder.cpp 80.00% <80.00%> (ø)
src/op/builder/einsum.cpp 98.91% <98.91%> (ø)

... and 110 files with indirect coverage changes

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov[bot] avatar Jul 31 '24 13:07 codecov[bot]

What's the status of this PR? Should anything be reviewed at this stage or a meeting held?

CharlieL7 avatar Sep 25 '24 00:09 CharlieL7

What's the status of this PR? Should anything be reviewed at this stage or a meeting held?

Work on this was put on hold due to higher-priority tasks on CK. However, this can be reviewed as is. What remains is the implementation of builders for the remaining operations.

mirza-halilcevic avatar Sep 25 '24 11:09 mirza-halilcevic

Is this superseded by the other op builder PRs? If so we can close @mirza-halilcevic

TedThemistokleous avatar Jun 14 '25 03:06 TedThemistokleous

Old API, have the new op builder PRs.

CharlieL7 avatar Aug 08 '25 21:08 CharlieL7