Samsung / ONE

On-device Neural Engine
Other
430 stars 157 forks source link

[Circle-OpSelector] Let's design Circle-OpSeletor Project #7596

Open Paran-Yu opened 3 years ago

Paran-Yu commented 3 years ago

Hi! We are team COSMOS from SSAFY(Samsung SW Academy For Youth). We will make Circle-OpSelector referring to issue #6648. This issue regards our plan on developing the Circle-OpSelector. We are open to all kinds of feedbacks, so please feel free to offer your opinion.

Options

1. Select from location numbers

(EDIT "1-3 5" -> "1-3,5", --op_ids -> --by_id)

./circle-opselector --by_id "1-3,5" input.circle output.circle

Then, circle-opselctor creates a model consisting of subgraphs that only contain nodes in the given op id list(1-3 means from 1 to 3). In the case above, nodes whose indices are { 1, 2, 3, 5 } will be selected.

2. Select from node names

(EDIT "Add_1-Sub_1 Concat_2" -> "Add_1,Sub_1,Concat_2", --op_names -> --by_name)

./circle-opselector --by_name "Add_1,Sub_1,Concat_2" input.circle output.circle

Then, circle-opselctor creates a model consisting of subgraphs that only contain nodes in the given op name list. In the case above, both nodes whose names are { Add_1, Sub_1, Concat_2 } and nodes between Add_1 and Sub_1 will be selected. (EDIT) Then, circle-opselctor creates a model consisting of subgraphs that only contain nodes in the given op name list. In the case above, both nodes whose names are { Add_1, Sub_1, Concat_2 } will be selected.

Todo

1. CLI

2. Test

3. Core

Directory Structure

(EDIT pass/ -> src/, delete singlepass, function1 -> OpSelector)

compiler/circle-opselector/
    |
    +-- driver/
    |     +-- Driver.cpp
    |
    +-- CMakeLists.txt
    +-- README.md
    +-- requires.cmake
    |
    +-- src/
          |
          +-- OpSelector.h
          +-- OpSelector.cpp
mhs4670go commented 3 years ago

It would be good if circle operators could be selected with only start node ~ end node. It selects the nodes connected between the start node and the end node. But seems very tricky:)

$ ./circle-opselector --op_between "3-7" input.circle output.circle

BTW, s in options could be removed. Just op_id and op_name will do the trick.

kevin622 commented 3 years ago

Thank you for your opinion, @mhs4670go ! We(team COSMOS) will take your great idea into consideration while developing the project.

glistening commented 3 years ago

BTW, s in options could be removed. Just op_id and op_name will do the trick.

+1. I like short names unless it makes it unclear.

I think op_ could be removed also. (i.w --id and --name).

Or if someone like to read command like English.

--by_name and --by_id could be a candidate.

glistening commented 3 years ago

It would be good if circle operators could be selected with only start node ~ end node. It selects the nodes connected between the start node and the end node. But seems very tricky:)

@mhs4670go As I understand, for a straight graph (i.e. mobilnet), we already can pass only start node and end node by using -. I guess you're thinking more complex graph (i.e. inceptionv3), which has two edges from one node. Is it right? Also I am curious what is the use case for your suggestion. I've been using select_operator.py (for tflite) without inconvenience. : )

glistening commented 3 years ago

For the tool name, I think circle-opselector is too long. I like to have a short name. I regret I've named nnpackage_run. nnpkg_run would be better. It will save my time and typing. : ) I think opselector can be used because our input model is likely to be assumed circle. Or we may cut both circle and tflite with one tool opselector. The file format could be automatically identified by its inputs extension.

dongyooncho commented 3 years ago

BTW, s in options could be removed. Just op_id and op_name will do the trick.

I think op_ could be removed also. If someone like to read command like English. --by_name and --by_id could be a candidate.

Thank you for your opinion. As advised, we will change the command to the following.

./opselector --by_id "1-3,5" input.circle output.circle
./opselector --by_name "Add_1-Sub_1,Concat_2" input.circle output.circle
llFreetimell commented 3 years ago

I think we should precisely define the definition of - :) - can be interpreted with following options

lemmaa commented 3 years ago

This is just an idea, not a requirement.. :)

I wish this tool could easily select the desired node for the graph. So far, everyone seems to be referring only to the + rule that adds the desired node. Conversely, can't a rule to exclude a specific node from among the already selected nodes be considered together?

In other words, the initially selected node is an empty set. Then I can add the desired node using various expressions. For example, +[all], +[1-5,7], +[15-:],+[Conv2D],+[Pool] would be such examples. Instead of +, other expressions such as --plus or --select may be used.

In addition, you can exclude some of the nodes that have already been added in the same way. Examples would be -[all], -[1-5,7], -[15-:],-[Conv2D],-[Pool]. Instead of -, you could use other expressions such as --minus, --deselect, etc.

These will be able to repeat each other in any way, and the end result will be how they are applied sequentially.

glistening commented 3 years ago

Please don't check checkbox before it get approved or reviewed. I think many checked items are still under working.

glistening commented 3 years ago

@lemmaa

This is just an idea, not a requirement.. :)

I wish this tool could easily select the desired node for the graph. So far, everyone seems to be referring only to the + rule that adds the desired node. Conversely, can't a rule to exclude a specific node from among the already selected nodes be considered together?

In other words, the initially selected node is an empty set. Then I can add the desired node using various expressions. For example, +[all], +[1-5,7], +[15-:],+[Conv2D],+[Pool] would be such examples. Instead of +, other expressions such as --plus or --select may be used.

During reviewing the handling of select and deselect in draft PR, I've found it assumes:

i) number will be considered as id, and ii) non-number is name.

I am not opposing this, it may be practical. However, I want to make sure we will accept this assumption ( i) ). As I understand, name is defined merely as string, it can be anything (including digit only string).

To avoid ambiguity, we may use --select-by-id and -select-by-name, --deselect-by-id (EDITED: name → id) and --deselect-by-name.

glistening commented 3 years ago

@lemmaa @Paran-Yu suggested --select and --deselect option's definition in #7667. Is it okay?

lemmaa commented 3 years ago

To avoid ambiguity, we may use --select-by-id and -select-by-name, ~--deselect-by-name~--deselect-by-id and --deselect-by-name.

I think this is a concise summary of my ideas.

One thing to add is that these 4 options can be used repeatedly many times in a single CLI command line.

That is, in processing this option, the node list must be updated by applying the options one by one sequentially while always maintaining the latest state of the node list.