Skip to content
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

add onnx-subgraph baseline code #14593

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

chenyx113
Copy link

single context of changes

related issue of: #14534
historical draft PR: #14563

Thanks for the review and good suggestions, I have updated the code as last review comments:

  1. move onnx-subgraph from compiler to tool path, so that it can have its own environment dependency
  2. refined readme.md, removed internal code name, and add step by step guideline for users
  3. removed unnecessary empty lines in some files
  4. move cpp files to src folder
  5. download the test onnx model files from web link

please help review, any suggestions and comments will be appreciated, thank you!

@chenyx113 chenyx113 marked this pull request as ready for review January 24, 2025 03:40
@seanshpark
Copy link
Contributor

@chenyx113 , there are too many files to review.
please split to "single context of changes" per PR,
like, introduce README, introduce CMakeLists with main file.
I'll request split PRs until PR itself is simple.

@chenyx113
Copy link
Author

@chenyx113 , there are too many files to review. please split to "single context of changes" per PR, like, introduce README, introduce CMakeLists with main file. I'll request split PRs until PR itself is simple.

@seanshpark Thanks for suggestion, because it is the first time uploading for the whole project, so it have many files. I understand it is difficult to review so many files at one PR.

but if split it to small changes, it will take many PRs for this submitting, is there any other suggestions for baseline code merge?

@chenyx113
Copy link
Author

@seanshpark
according to your suggestion, I checked our current code, and have plan as follow, please help check.
step 1: project set up with Readme.md only, to introduce the project feature and user guide
step 2: submit the onnx-subgraph parser lib related code with CMakeLists, it will include more cpp files, I think we should make sure the building success at each submitting
step 3: add main.cpp & config.json and show how to use the onnx-subgraph-parser lib APIs, and can generate onnx-subgraph exe
step 4: add test models downloading and pre-processing scripts for user testing
step 5: Add python files for split original onnx model to subgraph models accroding to onnx-subgraph parsing result
step 6: Add verification python code to compare the inference result from original onnx model and spitted subgraph models, to make sure the model splitting is right

step 2 will have more files, and other steps will be simple, hope for your opinion.

Thanks & BR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants