ONE icon indicating copy to clipboard operation
ONE copied to clipboard

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

Open Paran-Yu opened this issue 3 years ago • 12 comments

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

  • [x] Plan Options (#7596)
  • [x] Create Draft (#7600)
  • [x] Option Parsing
    • [x] --by_id, --by_name(#7777 )
    • [x] --select, --deselect

2. Test

  • [ ] Prepare Test Set(tflite file) for testing
  • [ ] Define selecting circle model process by shape of circle mode and command
  • [ ] Compare result of select_operator.py(select tflite) and opselector(select circle)
    • [ ] Select tflite and circle model(Converting .tflite to .circle)
    • [ ] Create golden value by selected tflite model
    • [ ] Create nnpackage by selected circle model
    • [ ] Compare two models using nnpkg-test
    • [ ] Create a shell script file to automate the above process
    • [ ] Manage cases which can't make golden value (#7630)
  • [ ] Test automation from random cases

3. Core

  • [ ] Checking connection of each nodes.
  • [ ] Copy selected nodes from a circle file
    • [ ] --by_id
    • [ ] --by_name
    • [ ] --select
    • [ ] --deselect
  • [ ] Create new circle file

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

Paran-Yu avatar Sep 01 '21 06:09 Paran-Yu

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.

mhs4670go avatar Sep 01 '21 07:09 mhs4670go

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

kevin622 avatar Sep 01 '21 07:09 kevin622

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 avatar Sep 02 '21 01:09 glistening

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 avatar Sep 02 '21 01:09 glistening

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.

glistening avatar Sep 02 '21 01:09 glistening

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

dongyooncho avatar Sep 02 '21 01:09 dongyooncho

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

  • (What I designed first in #6648) 2-5 means 2,3,4,5
  • (@mhs4670go suggestion) 2-5 means all nodes between node 2 and node 5
    • (@glistening pointed out) How we should handle for multiple branch in one node?

llFreetimell avatar Sep 02 '21 01:09 llFreetimell

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.

lemmaa avatar Sep 02 '21 16:09 lemmaa

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

glistening avatar Sep 13 '21 01:09 glistening

@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 avatar Sep 13 '21 01:09 glistening

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

glistening avatar Sep 13 '21 04:09 glistening

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.

lemmaa avatar Sep 13 '21 05:09 lemmaa