Skip to content

onnx-subgraph initial CMakeLists #14654

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

Merged
merged 3 commits into from
Feb 17, 2025

Conversation

chenyx113
Copy link
Contributor

@chenyx113 chenyx113 commented Feb 12, 2025

Brief introduction:
onnx-subgraph tool provides model auto partitioning of onnx model to several sub models by operator, performance and model size limitations, with the order and input / output names of sub models.

Purpose of current PR:
This is the initial PR of onnx-subgraph tool, only include the initial CMakeLists.txt in ./tools/onnx_subgraph, which is used to set up and debug the CI tools for auto building and testing new onnx-subgraph tool. the other codes and resources will be submitted step by step.

related issue of: #14534
historical full changes draft PR:
#14613

ONE-DCO-1.0-Signed-off-by: Youxin Chen yx113.chen@samsung.com

@chenyx113 chenyx113 marked this pull request as ready for review February 12, 2025 11:57
@seanshpark
Copy link
Contributor

I think you are not reading reviewers comments.
Plz check #14640 (comment)

@seanshpark
Copy link
Contributor

from #14640 (comment)

and rebase will introduce updating records in current PR

I don't think rebase will do that. Please check git commands.

@seanshpark
Copy link
Contributor

#14654 (comment) is not satisfied.

this is your commits: https://github.com/Samsung/ONE/pull/14654/commits
you need to update the first commit's message.

@chenyx113
Copy link
Contributor Author

#14654 (comment) is not satisfied.

this is your commits: https://github.com/Samsung/ONE/pull/14654/commits you need to update the first commit's message.

thanks for review, I update the comment now, please help check

@seanshpark
Copy link
Contributor

I update the comment now, please help check

I don't see any changes. Anyway I've triggered CIs.

@chenyx113
Copy link
Contributor Author

ONE-DCO-1.0-Signed-off-by: Youxin Chen yx113.chen@samsung.com

maybe some misunderstanding about comment body, I add it manually when creating PR refer to other PRs, is there any guide I can refer? sorry for making trouble

@seanshpark
Copy link
Contributor

is there any guide I can refer?

search for git commands.

I wrote to check https://github.com/Samsung/ONE/pull/14654/commits first item.
1/ have you checked the commit message ?
2/ do you know the problem ? if you think you do know, plz write it down so that I can double check

@chenyx113
Copy link
Contributor Author

is there any guide I can refer?

search for git commands.

I wrote to check https://github.com/Samsung/ONE/pull/14654/commits first item. 1/ have you checked the commit message ? 2/ do you know the problem ? if you think you do know, plz write it down so that I can double check

I understand it now, the signed information should be included in the commit msg body, rather than add it manually when creating PR. I need to do following steps locally to solve current issue

  1. git rebase -i HEAD~3, to make the first item of commits to be edit status

  2. git commit --amend to add the commit body
    CommitDate: Thu Feb 13 10:53:08 2025 +0800
    onnx-subgraph initial CMakeLists
    ONE-DCO-1.0-Signed-off-by: Youxin Chen yx113.chen@samsung.com

  3. git rebase --continue

please help check, thank you

@seanshpark
Copy link
Contributor

the signed information should be included in the commit msg body, rather than add it manually when creating PR

Yes :)

@chenyx113
Copy link
Contributor Author

@seanshpark
the comment body has been updated in the first commit item, please check. thank you

onnx-subgraph initial CMakeLists
ONE-DCO-1.0-Signed-off-by: Youxin Chen yx113.chen@samsung.com

@seanshpark
Copy link
Contributor

the comment body has been updated in the first commit item, please check. thank you

onnx-subgraph initial CMakeLists
ONE-DCO-1.0-Signed-off-by: Youxin Chen yx113.chen@samsung.com

you have added commit title, but with no commit body.
please add some explanation (single line is good, two or more is also ok, shorter is better)
plz check with other commits with git log and follow the patterns.

Initial committing of onnx-subgraph tool, only include the initial
CMakeLists.txt in ./tools/onnx_subgraph

ONE-DCO-1.0-Signed-off-by: Youxin Chen <yx113.chen@samsung.com>
@chenyx113
Copy link
Contributor Author

the comment body has been updated in the first commit item, please check. thank you
onnx-subgraph initial CMakeLists
ONE-DCO-1.0-Signed-off-by: Youxin Chen yx113.chen@samsung.com

you have added commit title, but with no commit body. please add some explanation (single line is good, two or more is also ok, shorter is better) plz check with other commits with git log and follow the patterns.

thanks for suggestion, I have refered other commits and update again

Copy link
Contributor

@seanshpark seanshpark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@seanshpark seanshpark merged commit 205191c into Samsung:master Feb 17, 2025
3 checks passed
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