[CFE] Introduce dredd test for TensorShape

[Hanjin-Choi]

What

Let's introduce dredd test for TensorShape.

Why

To test shape update of circle models.

Related : #10980

[pcs1265]

As you mentioned the related issue, I think this is limited tocircle2circle-dredd-recipe-test. Is that correct?
I didn't look around about other dredd tests. Should other dredd tests support shape testing?

[icodo98]

Before we support this, we have to discuss how to implement it.

1. add new dredd test like circle2circle, circle-quantizer, tf2circle, etc..
2. add new rule for shape inference in dredd-rule-lib
   • related : [CFE] dredd test support count for CircleConst #10014

Please share any ideas 😊

[shs-park]

There are pros and cons to both implementation methods.
Here's my opinion on them:

Create a new dredd test

• The amount of code to implement will be relatively large.
• Since we need to design the entire test, it could be more appealing as a project for SSAFY, especially for presentations.
• The shape inference will be tested separately, making it clearly distinct from existing tests.

Extending the existing dredd test

(e.g., circle2circle-dredd-recipe-test)

• You only need to properly extend the shape-related interface.
• The amount of code to implement could be smaller.
• You'll need to consider compatibility with the existing test models.
• It could be necessary to add shape tests to the Net_ series models that involve multiple operations, ensuring they can work together (e.g., operation fusion/folding and dynamic shape testing simultaneously).
  Alternatively, you can focus on testing just the shape aspect for now by keeping it well separated.

[icodo98]

Extending the existing dredd test

(e.g., circle2circle-dredd-recipe-test)

• You only need to properly extend the shape-related interface.
• The amount of code to implement could be smaller.
• You'll need to consider compatibility with the existing test models.
• It could be necessary to add shape tests to the Net_ series models that involve multiple operations, ensuring they can work together (e.g., operation fusion/folding and dynamic shape testing simultaneously).
  Alternatively, you can focus on testing just the shape aspect for now by keeping it well separated.

We discussed among ourselves and concluded that:

• Extending the existing dredd test would help maintain the project architecture.

However, during the design process, a question arose:

• It seems like we're adding rules that serve a different role.
  • The current circle2circle tests focus solely on operations like op_count and op_version, but these new rules appear to be related to tensors.

[zetwhite]

Aha, I didn't know that this issue opened last week 😢
I clearly understand the two options that @Samsung/ssafy_2024 discussed in today's meeting.

[shs-park]

It seems like we're adding rules that serve a different role.

• The current circle2circle tests focus solely on operations like op_count and op_version, but these new rules appear to be related to tensors.

👍

[icodo98]

• Extending the existing dredd test would help maintain the project architecture.

I made draft with this suggestion.
#14050
PTAL!
=)

[shs-park]

@seanshpark,

We are about to add the shape inference feature to the existing circle2circle_dredd_recipe_test,
and use the new tensor_shape Rule in this test.

For example,

# To check if dynamic dimension properly inferred

RULE    "VERIFY_FILE_FORMAT"      $(verify_file_format) '=' 1

RULE    "PAD_EXIST"               $(op_count PAD) '=' 1
RULE    "PAD_SHAPE"               $(tensor_shape ofm) '=' [1, -1, 7, 2]

Do you have any opinions?

[icodo98]

For example,

# To check if dynamic dimension properly inferred

RULE    "VERIFY_FILE_FORMAT"      $(verify_file_format) '=' 1

RULE    "PAD_EXIST"               $(op_count PAD) '=' 1
RULE    "PAD_SHAPE"               $(tensor_shape ofm) '=' [1, -1, 7, 2]

FYI) RULE "PAD_SHAPE" $(tensor_shape ofm) '=' [1, -1, 7, 2] does not work with the current code.
Currently, the rule only works if the arguments have exactly 4 elements, so there cannot be any spaces in between, such as [1, -1, 7, 2].
Therefore, at the moment, we are checking shapes in the form of 1,-1,7,2.

• Current code
  

  ONE/compiler/dredd-rule-lib/rule-lib.sh

  Lines 85 to 92 in 646c7f1

  	RULE()
  	{
  	argc_check $# 4
  	RULE_NAME=$1
  	ACTUAL=$2
  	COND=$3
  	EXPECTED=$4

[seanshpark]

$(tensor_shape ofm) '=' [1, -1, 7, 2]

Do you have any opinions?

Looks good to me :)

[seanshpark]

we are checking shapes in the form of 1,-1,7,2.

can "[1, -1, 7, 2]" be possible?

[icodo98]

can "[1, -1, 7, 2]" be possible?

I've tried with this method, but it fails.

• RULE (test.rule)

  RULE    "PAD_SHAPE"               $(tensor_shape ofm) '=' "[1, -1, 7, 2]"
  

• shell (compiler/dredd-rule-lib/rule-lib.sh)

  ACTUAL=`init_error_log ; \
            ${INSPECT_PROG_PATH} --tensor_shape ${COMPILED_FILE} | \
            awk -v tensor_name="$1" '{ if ($1 == tensor_name) {gsub("tensor_name ", "", $0); print $0}}'`

• circle-inspect output (circle-inspect)

   $ ./build/compiler/circle-inspect/circle-inspect --tensor_shape ./build/compiler/circle2circle-dredd-recipe-test/Inf_Pad_000.opt.circle 
    ifm [1, -1, 3, 2]
    padding_S32 [4, 2]
    ofm [1, -1, 7, 2]

• error log (build/compiler/circle2circle-dredd-recipe-test/Inf_Pad_000.error)

  arg count mismatch: actual = 8 vs expected = 4
  

I think if 1,-1,7,2 is acceptable, we can go with this format.

/cc @shs-park

[Hanjin-Choi]

I think if 1,-1,7,2 is acceptable, we can go with this format.

IMHO, it seems like [1,-1,7,2] would be more intuitive than without the brackets.

[shs-park]

IMHO, it seems like [1,-1,7,2] would be more intuitive than without the brackets.

👍

It might be a good idea to leave a comment in .rule file saying "Whitespace is not allowed inside shape expressions".

[icodo98]

It might be a good idea to leave a comment in .rule file saying "Whitespace is not allowed inside shape expressions".

👍

[Hanjin-Choi]

It might be a good idea to leave a comment in .rule file saying "Whitespace is not allowed inside shape expressions".

If we leave a comment in the .rule file, It seems we need to write it in each individual .rule file when we make a recipe.
How about leaving it in rule-lib.sh instead or dredd-rule-lib/README.md?

[shs-park]

How about leaving it in rule-lib.sh instead or dredd-rule-lib/README.md?

Sounds good =)

I think it would be ok to mention it in the Metrics supported section in the README.md.

[kyeong8139]

How can we add dredd-tests for static shape inference?

1. Create Inf_xxx_000

• To clearly separate shape inference
• However, this would duplicate the existing recipes

2. Add a rule to the existing recipes

• For example, add test.rule to Quantize_000 instead of creating Inf_Quantize_000

3. Don’t add a static shape inference test

[shs-park]

@kyeong8139,
In what cases is static shape inference necessary?
I'm asking because it seems like we've only discussed dynamic shape inference so far.

[kyeong8139]

In the unit tests, we test both static and dynamic shape inference.
So I was wondering if we also consider static shape inference in dredd-tests.

[shs-park]

If a pass that performs static shape inference is implemented in the future,
I think we can utilize the dredd test that we are currently making.

In that case, I think method 1 is better.

1. Create Inf_xxx_000

Is there any part you implemented this time that you would like to add a "static shape inference" test to? 😅

[kyeong8139]

If a pass that performs static shape inference is implemented in the future,
I think we can utilize the dredd test that we are currently making.

I agree with this opinion. IMHO, there's no need to implement it immediately.