add onnx-subgraph tool baseline code #14563

chenyx113 · 2025-01-18T03:27:53Z

related issue of: #14534
we have reported the progress status in internal discussion

provide the onnx auto subgraph tool baseline code
nncc configure, build, test has been done and run successfully
because it is a baseline code, there are more files added, after baseline, we will submit future commits <=50 lines

please help review, any suggestions is appreciated.

Thanks & BR

seanshpark · 2025-01-19T21:19:24Z

As I know, this is your full changes as a Draft PR, so changed to DRAFT.
I'll take some rough review, that may take some time.
It is OK to post PRs with single context of changes, that builds with success.
If you don't get what single context of changes is, I'll leave some comments when you post PRs.

seanshpark · 2025-01-19T21:20:50Z

some file names start with lower case and some with upper case. please use one convention.

seanshpark · 2025-01-19T21:21:38Z

compiler/onnx-subgraph/CMakeLists.txt

+# cmake version dependency
+cmake_minimum_required(VERSION 3.10)
+SET(CMAKE_BUILD_TYPE "Debug")
+SET(CMAKE_CXX_FLAGS_DEBUG "$ENV{CXXFLAGS} -O0 -Wall -g2 -ggdb")
+SET(CMAKE_CXX_FLAGS_RELEASE "$ENV{CXXFLAGS} -O3 -Wall")
+SET(CMAKE_CXX_STANDARD 17)
+
+project(onnx-subgraph-parser)


Suggested change

# cmake version dependency

cmake_minimum_required(VERSION 3.10)

SET(CMAKE_BUILD_TYPE "Debug")

SET(CMAKE_CXX_FLAGS_DEBUG "$ENV{CXXFLAGS} -O0 -Wall -g2 -ggdb")

SET(CMAKE_CXX_FLAGS_RELEASE "$ENV{CXXFLAGS} -O3 -Wall")

SET(CMAKE_CXX_STANDARD 17)

project(onnx-subgraph-parser)

seanshpark · 2025-01-19T21:23:42Z

compiler/onnx-subgraph/Readme.md

@@ -0,0 +1,48 @@
+## onnx_autosubgraphs


plz take look look at other modules README file and follow the convention.

seanshpark · 2025-01-19T21:23:57Z

compiler/onnx-subgraph/Readme.md

@@ -0,0 +1,48 @@
+## onnx_autosubgraphs
+in this project, we can support onnx AI model auto subgraph, the AI model can be  splitted by model size, operators etc, it help much for on-device AI acceleration, and has been verified on Rose-P NPU and Qualcomm DSP.


plz split lines with width > 100

IMHO, you shouldn't use internal code name!

seanshpark · 2025-01-19T21:25:04Z

compiler/onnx-subgraph/extract_onnx.py

+
+# Copyright (c) 2024 Samsung Electronics Co., Ltd. All Rights Reserved
+
+#
+
+# Licensed under the Apache License, Version 2.0 (the "License");


there are unnecessary empty lines.
please remove them like

Suggested change

# Copyright (c) 2024 Samsung Electronics Co., Ltd. All Rights Reserved

#

# Licensed under the Apache License, Version 2.0 (the "License");

# Copyright (c) 2024 Samsung Electronics Co., Ltd. All Rights Reserved

#

# Licensed under the Apache License, Version 2.0 (the "License");

seanshpark · 2025-01-19T21:26:41Z

compiler/onnx-subgraph/main.cpp

@@ -0,0 +1,76 @@
+


plz put source files into src folder.

seanshpark · 2025-01-19T21:27:34Z

plz run nncc format and correct formatting

seanshpark · 2025-01-19T23:43:40Z

compiler/onnx-subgraph/Readme.md

+    2. mkdir build & cd build
+    3. cmake .. & make


we don't allow building separate building in compiler/* modules
all compiler modules are built from infra/nncc/CMakeLists.txt
have you not read docs/howto/how-to-build-compiler.md ?

seanshpark · 2025-01-19T23:50:53Z

compiler/onnx-subgraph/Readme.md

+    1. edit the config.json as your needs
+       -> NPU_supported_ops mean operators that can be supported by NPU
+       -> CPU_supported_ops mean operators that can be supported by CPU


Is there any examples files that I can check with?

seanshpark · 2025-01-19T23:52:12Z

compiler/onnx-subgraph/Readme.md

@@ -0,0 +1,48 @@
+## onnx_autosubgraphs


It seems we have similar tool already with circle model, not ONNX.
But I think you can refer compiler/circle-partitioner/README.md file for explaining what and how to use the tool.

seanshpark · 2025-01-20T04:03:55Z

thinking about this tool and maintenance,

if we put inside as compiler/onnx-subgraph, this tool can be one of compiler tools, later can be distributed into Debian package, but needs to follow the existing rules.
CI does build check.
if we put inside as tools/onnx-subgraph, this is just tool share and can be somewhat independent from compiler or runtime, can have it's own build and execution environment.
Current CI doesn't do build check and has to add if necessary.

I am gradually thinking 2) case looks better. @chenyx113 and others, what do you guys think?

Anyway, we haven't done it in the past, but in the future, we can relocate any tools/* to compiler/* if requirements are met.

chenyx113 · 2025-01-20T05:13:54Z

thinking about this tool and maintenance,

if we put inside as compiler/onnx-subgraph, this tool can be one of compiler tools, later can be distributed into Debian package, but needs to follow the existing rules.
CI does build check.

if we put inside as tools/onnx-subgraph, this is just tool share and can be somewhat independent from compiler or runtime, can have it's own build and execution environment.
Current CI doesn't do build check and has to add if necessary.

I am gradually thinking 2) case looks better. @chenyx113 and others, what do you guys think?

Anyway, we haven't done it in the past, but in the future, we can relocate any tools/* to compiler/* if requirements are met.

@seanshpark
Thanks for your review and suggestions, I will update my code according to the comments.
and as the suggestion of "tools/onnx-subgraph,", I think it is a good idea, because our code is more like a general tool for AI model pre-processing. I will move current code to 'tool' and create new PR again, after testing and code format checking.

BR
Chen

chenyx113 · 2025-01-20T05:18:39Z

update the code according to comments, and change path from 'compiler' to 'tool', thanks for the kindly review and suggestions

add onnx-subgraph tool baseline code

a5ec1e7

seanshpark marked this pull request as draft January 19, 2025 21:19

seanshpark reviewed Jan 19, 2025

View reviewed changes

compiler/onnx-subgraph/main.cpp

@@ -0,0 +1,76 @@

Copy link

Contributor

seanshpark Jan 19, 2025

Choose a reason for hiding this comment

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

plz put source files into src folder.

seanshpark reviewed Jan 19, 2025

View reviewed changes

chenyx113 closed this Jan 20, 2025

chenyx113 deleted the onnx-subgraph branch January 23, 2025 13:51

chenyx113 mentioned this pull request Jan 24, 2025

add onnx-subgraph baseline code #14593

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add onnx-subgraph tool baseline code #14563

add onnx-subgraph tool baseline code #14563

chenyx113 commented Jan 18, 2025

seanshpark commented Jan 19, 2025 •

edited

Loading

seanshpark commented Jan 19, 2025

seanshpark Jan 19, 2025

seanshpark Jan 19, 2025

seanshpark Jan 19, 2025

seanshpark Jan 20, 2025

seanshpark Jan 19, 2025

seanshpark Jan 19, 2025

seanshpark commented Jan 19, 2025

seanshpark Jan 19, 2025

seanshpark Jan 19, 2025

seanshpark Jan 19, 2025

seanshpark commented Jan 20, 2025 •

edited

Loading

chenyx113 commented Jan 20, 2025

chenyx113 commented Jan 20, 2025

		@@ -0,0 +1,48 @@
		## onnx_autosubgraphs
		in this project, we can support onnx AI model auto subgraph, the AI model can be splitted by model size, operators etc, it help much for on-device AI acceleration, and has been verified on Rose-P NPU and Qualcomm DSP.

add onnx-subgraph tool baseline code #14563

add onnx-subgraph tool baseline code #14563

Conversation

chenyx113 commented Jan 18, 2025

seanshpark commented Jan 19, 2025 • edited Loading

seanshpark commented Jan 19, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seanshpark commented Jan 19, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seanshpark commented Jan 20, 2025 • edited Loading

chenyx113 commented Jan 20, 2025

chenyx113 commented Jan 20, 2025

seanshpark commented Jan 19, 2025 •

edited

Loading

seanshpark commented Jan 20, 2025 •

edited

Loading