-
Notifications
You must be signed in to change notification settings - Fork 139
Allow ONNX and TF modules optional #3972
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.
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
Open
apwojcik
wants to merge
71
commits into
develop
Choose a base branch
from
onnx_tf_optional
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
71 commits
Select commit
Hold shift + click to select a range
1463d65
Allow ONNX and TF modules optional
apwojcik 4cc170f
fix license
apwojcik 0dc8684
fix pyflakes
apwojcik bacde80
fix yapf
apwojcik 4ada011
Revert "fix yapf"
apwojcik 3ec4e30
fix python format
apwojcik 154bceb
fix pyflakes
apwojcik 6eeb27c
add config.h.in
apwojcik a8d4327
Merge branch 'develop' into onnx_tf_optional
apwojcik 31c95e1
change to MIGRAPHX_DISABLE_*
apwojcik dd77e86
remove 'guard_define'
apwojcik da302fa
revert blank line insert
apwojcik 344244b
correct config.h
apwojcik f0da7ef
change define01 to define
apwojcik a5678f0
remove pb_files
apwojcik 4aed614
add pb_files for tensorflow frontend
apwojcik fc6ce79
temp
apwojcik 0545f50
Merge branch 'develop' into onnx_tf_optional
apwojcik d9557c6
Fix compilation after merging #3905
apwojcik e02b184
fix license and formatting
apwojcik c6741b2
Merge branch 'fix_onnx_pb_files_windows' into onnx_tf_optional
apwojcik 2e2f5be
fix tidy
apwojcik 26375ed
Merge branch 'fix_onnx_pb_files_windows' into onnx_tf_optional
apwojcik 541dba0
use HAVE_* and migraphx_all_frontends
apwojcik ba75cd2
use inlude_guard()
apwojcik 56ff6ee
Merge branch 'fix_onnx_pb_files_windows' into onnx_tf_optional
apwojcik f76b1be
fix resource files for api tests
apwojcik 3e9f26e
move Embed to master CMake
apwojcik 6eb730a
move Embed to main CMakeList.txt
apwojcik 2d3ecfd
fix compile with hipBLASLt OFF
apwojcik 652aaf5
changed to global property
apwojcik 62f6e7c
add brief_doc
apwojcik 2c37a50
Revert "move Embed to main CMakeList.txt"
apwojcik 0bd61fb
Revert "changed to global property"
apwojcik aa0a3ac
fix building Python modules for virutal env
apwojcik 2f62cad
Merge branch 'develop' into onnx_tf_optional
apwojcik b51167e
Merge branch 'fix_onnx_pb_files_windows' into onnx_tf_optional
apwojcik 9f56376
fix docker issue
apwojcik 402ffd3
fix linking python
apwojcik 00e930d
Merge branch 'develop' into onnx_tf_optional
apwojcik d6d0347
revert Embed from master CMakeLists.txt
apwojcik 0db637a
Fix Python virtual env issue
apwojcik 456aa20
Merge branch 'fix_python_modules_venv_windows' into onnx_tf_optional
apwojcik c7426f7
some changes
apwojcik 8d5097f
hide console window when spawning a process
apwojcik 3458407
Merge branch 'develop' into onnx_tf_optional
apwojcik bc895f1
Merge branch 'develop' into onnx_tf_optional
apwojcik fcccf61
Merge branch 'develop' into onnx_tf_optional
apwojcik b391c58
move generate to the API CMakeLists.txt
apwojcik 012dc9d
remove add_subdirectory(tools) from main CMakeLists.txt
apwojcik 9d3a656
fix dependencies to generate target
apwojcik 5fc4c37
incorporate review feeedback
apwojcik 92839f7
update license
apwojcik e2533b5
make clang-format optional
apwojcik 7f0537a
fix pyflakes
apwojcik dfaefb3
Merge branch 'develop' into onnx_tf_optional
apwojcik 71f244a
fix tidy
apwojcik c59bd23
Revert "fix tidy"
apwojcik 546e647
Merge branch 'develop' into onnx_tf_optional
apwojcik e6fb3db
Merge branch 'develop' into onnx_tf_optional
apwojcik 97a9c7e
Merge branch 'develop' into onnx_tf_optional
apwojcik bfabef7
Merge branch 'develop' into onnx_tf_optional
apwojcik bf21796
Merge branch 'develop' into onnx_tf_optional
apwojcik d11bc28
update license
apwojcik dbc19ba
Merge branch 'develop' into onnx_tf_optional
apwojcik f7d7d5f
Merge branch 'develop' into onnx_tf_optional
apwojcik 033900b
Merge branch 'develop' into onnx_tf_optional
apwojcik 1b388f5
Merge branch 'develop' into onnx_tf_optional
apwojcik dc1ecda
Merge branch 'develop' into onnx_tf_optional
apwojcik 51c0384
Update src/py/migraphx_py.cpp
apwojcik 975b157
Update src/py/migraphx_py.cpp
apwojcik File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,7 @@ | ||
| ##################################################################################### | ||
| # The MIT License (MIT) | ||
| # | ||
| # Copyright (c) 2015-2025 Advanced Micro Devices, Inc. All rights reserved. | ||
| # Copyright (c) 2015-2026 Advanced Micro Devices, Inc. All rights reserved. | ||
| # | ||
| # Permission is hereby granted, free of charge, to any person obtaining a copy | ||
| # of this software and associated documentation files (the "Software"), to deal | ||
|
|
@@ -22,9 +22,37 @@ | |
| # THE SOFTWARE. | ||
| ##################################################################################### | ||
|
|
||
| add_library(migraphx_c | ||
| api.cpp | ||
| ) | ||
| find_package(Python 3 REQUIRED COMPONENTS Interpreter) | ||
| find_program(CLANG_FORMAT clang-format PATHS /opt/rocm/llvm ENV HIP_PATH PATH_SUFFIXES bin) | ||
|
|
||
| set(TOOLS_DIR "${PROJECT_SOURCE_DIR}/tools") | ||
| set(BIN_DIR "${PROJECT_BINARY_DIR}/src/api") | ||
| set(SRC_DIR "${PROJECT_SOURCE_DIR}/src/api") | ||
|
|
||
| set(GENERATE_COMMAND ${Python_EXECUTABLE} generate.py) | ||
| if (CLANG_FORMAT) | ||
| list(APPEND GENERATE_COMMAND -f ${CLANG_FORMAT}) | ||
| endif() | ||
| if(MIGRAPHX_ENABLE_ONNX) | ||
| list(APPEND GENERATE_COMMAND -Denable_onnx) | ||
| endif() | ||
| if(MIGRAPHX_ENABLE_TENSORFLOW) | ||
| list(APPEND GENERATE_COMMAND -Denable_tensorflow) | ||
| endif() | ||
| list(APPEND GENERATE_COMMAND -o ${PROJECT_BINARY_DIR}/src) | ||
|
|
||
| add_custom_command( | ||
| OUTPUT ${BIN_DIR}/api.cpp | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This isnt the only output file.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Or should this just run api.py instead of generate.py? |
||
| COMMAND ${GENERATE_COMMAND} | ||
| DEPENDS ${TOOLS_DIR}/generate.py ${TOOLS_DIR}/api.py ${TOOLS_DIR}/te.py ${SRC_DIR}/migraphx.py | ||
| WORKING_DIRECTORY ${TOOLS_DIR} | ||
| COMMENT "Generate MIGraphX C API source files...") | ||
|
|
||
| add_custom_target(generate DEPENDS ${BIN_DIR}/api.cpp) | ||
|
|
||
| add_library(migraphx_c ${BIN_DIR}/api.cpp) | ||
| add_dependencies(migraphx_c generate) | ||
|
|
||
| set_target_properties(migraphx_c PROPERTIES EXPORT_NAME c) | ||
| migraphx_generate_export_header(migraphx_c DIRECTORY migraphx/api) | ||
|
|
||
|
|
@@ -37,7 +65,7 @@ if(BUILD_TESTING) | |
| endif() | ||
|
|
||
| rocm_clang_tidy_check(migraphx_c) | ||
| target_link_libraries(migraphx_c PRIVATE migraphx migraphx_tf migraphx_onnx) | ||
| target_link_libraries(migraphx_c PRIVATE migraphx migraphx_all_frontends) | ||
| target_link_libraries(migraphx_c PUBLIC migraphx_version) | ||
|
|
||
| rocm_install_targets( | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This changes the generated files to be in the build directory instead of the source directory, but you need to remove all the generated source files that are currently committed.
Although, I prefer to have the source committed especially for the API as it makes it easier to view how API changes might change ABI. I understand how it makes it easier for these custom builds though. Perhaps there si a better way to handle this. I dont know what other team members think of this change @CharlieL7 @shivadbhavsar @kahmed10.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's OK to not have the source committed. Having both the generation files and the source has led to developers directly editing generated files.