add onnx-subgraph baseline code

[chenyx113]

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!

[seanshpark]

@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]

@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]

@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

[seanshpark]

according to your suggestion, I checked our current code, and have plan as follow, please help check.

I don't know the details of the plan. The basic rule is to put single context of changes and not much (not more than 50 lines) to review or your PR will have too many comments that will not be appropriate for you and the reviewers to communicate.

[seanshpark]

Q) is current PR has full changes? or is it a part ?
title has baseline and I assume there are more.
As I wrote on the e-mail, please put full changes so that when I see codes that I don't understand the reason, I can reference the full changes, not to ask dumb questions.

[chenyx113]

Q) is current PR has full changes? or is it a part ? title has baseline and I assume there are more. As I wrote on the e-mail, please put full changes so that when I see codes that I don't understand the reason, I can reference the full changes, not to ask dumb questions.

@seanshpark
it is the full changes, includes all of the code of onnx-subgraph tool, can be tested as the guideline in the Readme.md.
thank you:)

[chenyx113]

Q) is current PR has full changes? or is it a part ? title has baseline and I assume there are more. As I wrote on the e-mail, please put full changes so that when I see codes that I don't understand the reason, I can reference the full changes, not to ask dumb questions.

@seanshpark
because it is the first submitting, and include full changes in this PR, so that you can check and test our code as guideline in the readme.md, if follow "The basic rule is to put single context of changes and not much (not more than 50 lines) ", it is difficult for me to split so many files, hope for your opinion, thank you :)

[seanshpark]

it is difficult for me to split so many files

what is the difficulties?

[seanshpark]

Q) do you have a plan to include this tool in compiler Debian package release (like in https://github.com/Samsung/ONE/releases/tag/1.29.0) ?
if not, what is your final usage scenario ?

[chenyx113]

it is difficult for me to split so many files

what is the difficulties?

@seanshpark
it is a new added tool for onnx model subgraph, and doesn't has dependency with other existing modules, if submitting full changes in one PR, then it can be tested and verified very easily, otherwise, it can be tested after many commits later.

there are 19 files in current PR, and include 7000+ lines code & comment.
if follow the rule "<=50 lines", then it needs more than 100 commits for submitting, which will take much time for me and reviewer

[chenyx113]

Q) do you have a plan to include this tool in compiler Debian package release (like in https://github.com/Samsung/ONE/releases/tag/1.29.0) ? if not, what is your final usage scenario ?

previously, I have merged our code to compiler, and got comment:
#14563 (comment)

so I changed building scripts and merge code to ./tools
I think our code is a general tool for onnx subgraph, can be used for customized model splitting, according to your suggestion, also with our own discussion, I think it is better in ./tools rather than in ./compiler

thank you:)

[seanshpark]

if submitting full changes in one PR,

I assume you want to land the code as is?
As a maintainer, I cannot accept that. Reviewing all the code takes too much energy that makes me difficult.

then it can be tested and verified very easily, otherwise, it can be tested after many commits later.

I assume this PR already has tested codes.
I'm planning to prepare a Workflow with compile and test for this tool.
We recommend a PR should have a unit test for that change but not always.
When the Workflow is ready, CI will give some reports with the code.

more than 100 commits for submitting, which will take much time for me and reviewer

Yes. I expect that. But I'd expect with this PR, will take more time reviewing and you explaining why... and applying those comments and so on.
Anyway I've never reviewed or accepted huge PRs.

[seanshpark]

previously, I have merged our code to compiler, and got comment:
#14563 (comment)

So you accept as individual tool. That is OK.

[seanshpark]

I've approved to run the CI, and I think only format checker will run.
Please note that all test must pass for the review for PRs.

[seanshpark]

I'm planning to prepare a Workflow with compile and test for this tool.

related issue is #14567

[chenyx113]

I'm planning to prepare a Workflow with compile and test for this tool.

related issue is #14567

I've approved to run the CI, and I think only format checker will run. Please note that all test must pass for the review for PRs.

yes, I found some format checking issue, and I will correct related issues and submit again, thank you for your kindly guide

[chenyx113]

new PR has been submitted: #14613