[DRAFT][dredd] Introduce dredd test for TensorShape #14050

icodo98 · 2024-09-23T01:49:45Z

Introduce new dredd-rule for tensors shape.

ONE-DCO-1.0-Signed-off-by: JuYoung Lee rsb98759@gmail.com

icodo98 · 2024-09-23T01:50:21Z

related : #13998

res/TensorFlowLiteRecipes/Pad_002/test.rule

icodo98 · 2024-09-24T00:47:26Z

@nnfw-bot test nncc-debug nncc-release

kyeong8139 · 2024-09-23T02:00:09Z

res/TensorFlowLiteRecipes/Pad_002/test.recipe

@@ -0,0 +1,33 @@
+# padding with dynamic shape, others same as Pad_000


This recipe and rule ard designed for shape Inference.
How about changing Pad_002 directory to Inf_Pad_00x to separate it from other tests like Net_Conv_Add_00x ?

How about changing Pad_002 directory to Inf_Pad_00x to separate it from other tests like Net_Conv_Add_00x ?

Nice idea!

I'll change it.

Introduce new dredd-rule for check tensors shape. ONE-DCO-1.0-Signed-off-by: JuYoung Lee rsb98759@gmail.com

ONE-DCO-1.0-Signed-off-by: JuYoung Lee rsb98759@gmail.com

shs-park

It seems to work well!
I left a few minor comments.

shs-park · 2024-09-24T07:35:22Z

compiler/circle-inspect/src/Dump.cpp

+      const auto tensor = tensors->Get(i);
+      auto shape = tensor->shape_signature() ? tensor->shape_signature() : tensor->shape();
+      os << reader.tensor_name(tensor) << " ";
+      for (int8_t i = 0; i < shape->size(); i++)


It's not a must, but most machines do calculations in 32-bit units, so there's probably no need to reduce it to 8-bit in for loop.

Suggested change

for (int8_t i = 0; i < shape->size(); i++)

for (uint32_t i = 0; i < shape->size(); i++)

It's not a must, but most machines do calculations in 32-bit units, so there's probably no need to reduce it to 8-bit in for loop.

I'll update it.

shs-park · 2024-09-24T07:51:09Z

res/TensorFlowLiteRecipes/Inf_Pad_000/test.rule

+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


The 'shape' expression 1,-1,7,2 is fine now, but it would be better to express it in a more formalized form like [1, -1, 7, 2].
It would be better if it could handle spaces between commas as well.

Suggested change

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

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

from ##13998 (comment)

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

Oh, thanks for the description.
How about using a quoted string, i.e., "[1, -1, 7, 2]"?
If it's difficult, the current implementation is not bad either =)

shs-park · 2024-09-24T07:55:12Z

res/TensorFlowLiteRecipes/Inf_Pad_000/test.rule

@@ -0,0 +1,6 @@
+# To check if dynamic dimension properly infered


typo
=)

Suggested change

# To check if dynamic dimension properly infered

# To check if dynamic dimension properly inferred

typo =)

Sorry, I'll fix it.

shs-park · 2024-09-24T07:56:45Z

compiler/circle2circle-dredd-recipe-test/test.lst

@@ -96,6 +96,7 @@ Add(PadV2_001 PASS substitute_padv2_to_pad)
 Add(Softmax_001 PASS decompose_softmax)
 Add(Softmax_002 PASS decompose_softmax)
 Add(StridedSlice_003 PASS substitute_strided_slice_to_reshape)
+Add(Inf_Pad_000 PASS)


Please sort in alphabetical order

Please sort in alphabetical order

Instead of sorting, what about separate Inf_ tests?

Suggested change

Add(Inf_Pad_000 PASS)

# Shape Inference test

Add(Inf_Pad_000 PASS)

, what about separate Inf_ tests?

👍

change rule shape to be enclosed in brackets change type to uint32_t seperate inference test list ONE-DCO-1.0-Signed-off-by: JuYoung Lee rsb98759@gmail.com

kyeong8139 · 2024-09-25T00:47:02Z

res/TensorFlowLiteRecipes/Inf_Pad_000/test.rule

+
+RULE    "VERIFY_FILE_FORMAT"      $(verify_file_format) '=' 1
+
+RULE    "PAD_EXIST"               $(op_count PAD) '=' 1


IMHO, PAD_EXIST is not related to SHAPE_INFERENCE.
Why is this necessary?

Why is this necessary?

You're right.

ONE-DCO-1.0-Signed-off-by: sunki <ajsthfldu@outlook.com>

remove unneccesary rule for shape inference. ONE-DCO-1.0-Signed-off-by: JuYoung Lee rsb98759@gmail.com

icodo98 added DRAFT A draft issue or PR for sharing one's current working status and discussion. SSAFY labels Sep 23, 2024

icodo98 requested a review from a team September 23, 2024 01:49

icodo98 mentioned this pull request Sep 23, 2024

[CFE] Introduce dredd test for TensorShape #13998

Open

qsunki reviewed Sep 23, 2024

View reviewed changes

res/TensorFlowLiteRecipes/Pad_002/test.rule Outdated Show resolved Hide resolved

icodo98 changed the title ~~[dredd] Introduce dredd test for TensorShape~~ [DRAFT][dredd] Introduce dredd test for TensorShape Sep 24, 2024

kyeong8139 reviewed Sep 24, 2024

View reviewed changes

icodo98 added 2 commits September 24, 2024 15:37

[dredd] Introduce new rule for tensorshape

7bf462c

Introduce new dredd-rule for check tensors shape. ONE-DCO-1.0-Signed-off-by: JuYoung Lee rsb98759@gmail.com

[dredd] add comma

2178227

ONE-DCO-1.0-Signed-off-by: JuYoung Lee rsb98759@gmail.com

icodo98 force-pushed the draft-dredd branch from bfb9c6f to 2178227 Compare September 24, 2024 06:44

[dredd] add prefix Inf_ to recipe

98cd197

ONE-DCO-1.0-Signed-off-by: JuYoung Lee rsb98759@gmail.com

shs-park reviewed Sep 24, 2024

View reviewed changes

[dredd] apply suggestions

9242803

change rule shape to be enclosed in brackets change type to uint32_t seperate inference test list ONE-DCO-1.0-Signed-off-by: JuYoung Lee rsb98759@gmail.com

icodo98 force-pushed the draft-dredd branch from 51db89a to 9242803 Compare September 25, 2024 00:41

kyeong8139 reviewed Sep 25, 2024

View reviewed changes

qsunki force-pushed the draft-dredd branch from 145974b to 9242803 Compare September 25, 2024 02:27

[docs] Update dredd-rule-lib README

2354adc

ONE-DCO-1.0-Signed-off-by: sunki <ajsthfldu@outlook.com>

qsunki force-pushed the draft-dredd branch from 4c897e3 to 2354adc Compare September 25, 2024 04:14

[dredd] fix rule

a551138

remove unneccesary rule for shape inference. ONE-DCO-1.0-Signed-off-by: JuYoung Lee rsb98759@gmail.com

icodo98 closed this Oct 2, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DRAFT][dredd] Introduce dredd test for TensorShape #14050

[DRAFT][dredd] Introduce dredd test for TensorShape #14050

icodo98 commented Sep 23, 2024

icodo98 commented Sep 23, 2024 •

edited by jongwonyang

Loading

icodo98 commented Sep 24, 2024

kyeong8139 Sep 23, 2024

icodo98 Sep 24, 2024

shs-park left a comment

shs-park Sep 24, 2024

icodo98 Sep 24, 2024

shs-park Sep 24, 2024

icodo98 Sep 24, 2024

shs-park Sep 24, 2024

shs-park Sep 24, 2024

icodo98 Sep 24, 2024 •

edited

Loading

shs-park Sep 24, 2024

icodo98 Sep 24, 2024

shs-park Sep 24, 2024

kyeong8139 Sep 25, 2024

icodo98 Sep 25, 2024 •

edited

Loading

		@@ -0,0 +1,33 @@
		# padding with dynamic shape, others same as Pad_000

	for (int8_t i = 0; i < shape->size(); i++)
	for (uint32_t i = 0; i < shape->size(); i++)

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

	RULE()
	{
	argc_check $# 4

	RULE_NAME=$1
	ACTUAL=$2
	COND=$3
	EXPECTED=$4

		@@ -0,0 +1,6 @@
		# To check if dynamic dimension properly infered

	Add(Inf_Pad_000 PASS)
	# Shape Inference test
	Add(Inf_Pad_000 PASS)


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

		RULE "PAD_EXIST" $(op_count PAD) '=' 1

[DRAFT][dredd] Introduce dredd test for TensorShape #14050

[DRAFT][dredd] Introduce dredd test for TensorShape #14050

Conversation

icodo98 commented Sep 23, 2024

icodo98 commented Sep 23, 2024 • edited by jongwonyang Loading

icodo98 commented Sep 24, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shs-park left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

icodo98 Sep 24, 2024 • edited Loading

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

icodo98 Sep 25, 2024 • edited Loading

Choose a reason for hiding this comment

icodo98 commented Sep 23, 2024 •

edited by jongwonyang

Loading

icodo98 Sep 24, 2024 •

edited

Loading

icodo98 Sep 25, 2024 •

edited

Loading