[Frontend][PaddlePaddle] Add 10+ operators for PaddlePaddle

[jiangjiajun]

This pull request is part of #9102 hope this will bring some help to review

@AndrewZhaoLuo
Thanks for contributing to TVM! Please refer to guideline https://tvm.apache.org/docs/contribute/ for useful information and tips. After the pull request is submitted, please request code reviews from Reviewers by @ them in the pull request thread.

[mbrookhart]

This is really big, so it's hard to catch all of the edge cases in a review, but it looks okay, with the standard caveats that dynamic support might not be fully there yet. Not seeing anything I disapprove of, but I'd like to get more eyes on it before approving.

[mbrookhart]

Just as a heads up, I supported SAME padding in the ONNX frontend with dynamic shapes here:

tvm/python/tvm/relay/frontend/onnx.py

Lines 412 to 472 in d0c6ca5

	defautopad(
	data,
	strides,
	kernel_shape,
	dilations,
	ndim,
	pad_type="constant",
	deconv=False,
	mode="SAME_UPPER",
	pad_value=0.0,
	):
	"""
	Perform autopadding with dynamic input shapes
	"""
	# get attributes as constants
	strides=_op.const(np.array(strides), dtype="int64")
	dilated_kernel_shape=_op.const(
	np.array(
	[(kernel-1) *dilation+1forkernel, dilationinzip(kernel_shape, dilations)]
	),
	dtype="int64",
	)
	# get input shape
	shape=_op.strided_slice(shape_of(data, dtype="int64"), [2], [ndim])
	# set up integer constants
	zero=_op.const(0, dtype="int64")
	one=_op.const(1, dtype="int64")
	two=_op.const(2, dtype="int64")
	# Calculate total padding
	mod=_op.mod(shape, strides)
	left=_op.maximum(dilated_kernel_shape-strides, zero)
	right=_op.maximum(dilated_kernel_shape-mod, zero)
	total_pad=_op.where(_op.equal(mod, zero), left, right)
	ifdeconv:
	total_pad=_op.const(np.array(kernel_shape), dtype="int64") -one-total_pad
	# split total padding into before and after
	pad_before=_op.floor_divide(total_pad, two)
	pad_after=total_pad-pad_before
	# combine
	if"LOWER"inmode:
	pad=_op.concatenate(
	[_op.reshape(pad_after, [-1, 1]), _op.reshape(pad_before, [-1, 1])], axis=1
	)
	else:
	pad=_op.concatenate(
	[_op.reshape(pad_before, [-1, 1]), _op.reshape(pad_after, [-1, 1])], axis=1
	)
	# pad N and C with zeros
	pad=_op.concatenate([_op.const(np.zeros([2, 2], dtype="int64"), dtype="int64"), pad], axis=0)
	ifisinstance(pad_value, (float, int)):
	pad_value=_op.const(pad_value)
	return_op.nn.pad(data, fold_constant(pad), pad_value, pad_type)

It's fairly complicated, I'm totally cool if you want to punt on that until you need it.

[jiangjiajun]

Thanks, this will solve a big problem! But to avoid making this pull request more complicated to review, let's left this for the next pull request.

[AndrewZhaoLuo]

Some initial comments. I've gotten up to convert_fill_constant will review the rest later. Agree with mbrookhart. This is pretty big so we might need more eyes.

Can you send the most up to date english docs btw?

Also, does PaddlePaddle support operator versioning? How will you handle API changes in the future?

[AndrewZhaoLuo]

Can you use python/tvm/relay/frontend/common.py::infer_shape?

[jiangjiajun]

We have referred ONNX frontend, this function also comes from there https://github.com/apache/tvm/blob/main/python/tvm/relay/frontend/onnx.py#L1411
It's a little different from common::infer_shape

[AndrewZhaoLuo]

Can you use python/tvm/relay/frontend/common.py::infer_value?

[jiangjiajun]

Done. I just found there's try_infer_value in common.py, this function is removed.

[AndrewZhaoLuo]

In general Docstrings should be complete sentences. End with a period and capitalize the first letter.

E.g. "Calculate the paddings size."

Please fix the other docstrings

[jiangjiajun]

Done

[AndrewZhaoLuo]

Do you have a link to the english docs?

https://www.paddlepaddle.org.cn/documentation/docs/en/1.8/api/layers/argmax.html

Doesn't seem to have some of the attributes listed in the op

[jiangjiajun]

Now the latest version is 2.1, API documents: https://www.paddlepaddle.org.cn/documentation/docs/en/api/paddle/argmax_en.html#argmax

Follow the API definition code, we can find there's some attributes not list in the API's parameters
https://github.com/PaddlePaddle/Paddle/blob/release/2.1/python/paddle/tensor/search.py#L179

 attrs['keepdims'] = keepdim
attrs['axis'] = axis
attrs['flatten'] = flatten
attrs['dtype'] = var_dtype
helper.append_op(
type='arg_max', inputs={'X': x}, outputs={'Out': [out]}, attrs=attrs)
out.stop_gradient = True

[AndrewZhaoLuo]

A lot of the logic in argmin and argmax is similar. Refactor to combine the two.

[jiangjiajun]

Done

[AndrewZhaoLuo]

consider using _op.gather on the out_indices

[jiangjiajun]

Done

[AndrewZhaoLuo]

nit: out_indices

[jiangjiajun]

Done

[AndrewZhaoLuo]

You might want to note the semantics of paddle paddle's dot operator. Namely how it also operates on 2d-vectors (and hence why axis=[-1]).

I have not seen this elsewhere

[jiangjiajun]

paddle.dot : https://www.paddlepaddle.org.cn/documentation/docs/en/api/paddle/dot_en.html

It's similar with torch.dot, while torch.dot only supports 1D tensor.
In PaddlePaddle, inputs should be both 1D or 2D tensor. When it is 2d, the first dimension of this matrix is the batch dimension.

For clarify, I also put this explanation in code

[jiangjiajun]

Hi, @AndrewZhaoLuo@mbrookhart
Thanks for reviewing this PR.

1. PaddlePaddle now provides English document for API , there's no document describe operators, we are supporting this operators mostly by refer to its cpp code or test code, like crop_op.h or test_crop_tensor.py, but I think it's necessary to provide such documents, I'll try to push this within the PaddlePaddle team in the next quarter. For now, if you have any question about the operators, just comment in this pr, I'll try to make explanation here.
2. Like other framework, PaddlePaddle has needs to upgrade operators, this will add or delete some parameters for the operator, and also bring a new operator name, like squeeze2 or multiclass_nms3. Currently, we create a convert function for all the different versions of operator, but different versions of operator are both list in the _convert_map.

[jiangjiajun]

This PR is still too big I think considering most work here is unrelated to each other. Can you remove operators from this PR until you are down to ~+300 loc?

Just so you know, all ops above convert_fill_constant I have taken a look at so if you reduce this PR down to those changes only the review process can go a lot faster

Hi, @AndrewZhaoLuo
All the modifications under convert_fill_constant are removed .
Still lack of lots of pull requests to finish my work, I'll try to classify these pull requests to make reviewing faster

[AndrewZhaoLuo]

Coming together well. I think I am ok with merging this in the current state as part of a more experimental frontend, but would like another pair of eyes on this.

Still a few comments which will make this better:

1. Can you add more substantial test cases to your tests? E.g. different input shapes and those of different ranks at least for some of the relevant ops. I feel that some issues might come to light from this. Do not worry about PR size anymore

2. Please add type annotations to functions for this and future PRs. e.g.

def myFunc(a: int, b: List[string]) -> None:
...

[AndrewZhaoLuo]

I believe you might run into a similar issue as https://github.com/apache/tvm/blob/main/python/tvm/relay/frontend/onnx.py#L2257

[jiangjiajun]

PaddlePaddle's expand_as doesn't support multi-directional broadcasting, so this problem will not happen in PaddlePaddle frontend

[AndrewZhaoLuo]

Should describe padding size for what

[jiangjiajun]

Done

[jiangjiajun]

Coming together well. I think I am ok with merging this in the current state as part of a more experimental frontend, but would like another pair of eyes on this.

Still a few comments which will make this better:

1. Can you add more substantial test cases to your tests? E.g. different input shapes and those of different ranks at least for some of the relevant ops. I feel that some issues might come to light from this. Do not worry about PR size anymore
2. Please add type annotations to functions for this and future PRs. e.g.

def myFunc(a: int, b: List[string]) -> None: ...

• More cases have been added in tests, but only for the new operators in this pull request. I will send another pull request for the previous operators.
• Type annotation is a good code habit, but for the function like convert_dot(g : GraphProto, op : paddle.fluid.framework.operators, block: paddle.fluid.framework.Block), the type annotation will bring dependency of paddlepaddle for TVM, I noticed that all the frontends putting framework importing in from_xxx function to avoid strong dependency for TVM.

[AndrewZhaoLuo]

Shame about the typing, you can do forward references like:

def g(f:"paddle.paddleblahblah.blah"): -->:

But eh it's not the end of the world to be untyped since the rest of the frontends are like that. Just one comment about test case sizes.

We'll need another approver though. @mbrookhart ?

[AndrewZhaoLuo]

Please reduce the size of your test cases to something smaller e.g. less than 256 total elements (totally arbitrary, just as small as possible while still accomplishing the test)

[jiangjiajun]

Done. I guess the limit on the number of elements is to reduce the cost time of testing?

[AndrewZhaoLuo]

LGTM

[jiangjiajun]

@junrushao1994 Hi, could you help to merge this pull request?

[junrushao]

Thanks @AndrewZhaoLuo for the review! Thanks @jiangjiajun for the PR!

[jiangjiajun]

This pull request is part of #9102 hope this will bring some help to review

@AndrewZhaoLuo
Thanks for contributing to TVM! Please refer to guideline https://tvm.apache.org/docs/contribute/ for useful information and tips. After the pull request is submitted, please request code reviews from Reviewers by @ them in the pull request thread.

[mbrookhart]

This is really big, so it's hard to catch all of the edge cases in a review, but it looks okay, with the standard caveats that dynamic support might not be fully there yet. Not seeing anything I disapprove of, but I'd like to get more eyes on it before approving.

[mbrookhart]

Just as a heads up, I supported SAME padding in the ONNX frontend with dynamic shapes here:

tvm/python/tvm/relay/frontend/onnx.py

Lines 412 to 472 in d0c6ca5

	defautopad(
	data,
	strides,
	kernel_shape,
	dilations,
	ndim,
	pad_type="constant",
	deconv=False,
	mode="SAME_UPPER",
	pad_value=0.0,
	):
	"""
	Perform autopadding with dynamic input shapes
	"""
	# get attributes as constants
	strides=_op.const(np.array(strides), dtype="int64")
	dilated_kernel_shape=_op.const(
	np.array(
	[(kernel-1) *dilation+1forkernel, dilationinzip(kernel_shape, dilations)]
	),
	dtype="int64",
	)
	# get input shape
	shape=_op.strided_slice(shape_of(data, dtype="int64"), [2], [ndim])
	# set up integer constants
	zero=_op.const(0, dtype="int64")
	one=_op.const(1, dtype="int64")
	two=_op.const(2, dtype="int64")
	# Calculate total padding
	mod=_op.mod(shape, strides)
	left=_op.maximum(dilated_kernel_shape-strides, zero)
	right=_op.maximum(dilated_kernel_shape-mod, zero)
	total_pad=_op.where(_op.equal(mod, zero), left, right)
	ifdeconv:
	total_pad=_op.const(np.array(kernel_shape), dtype="int64") -one-total_pad
	# split total padding into before and after
	pad_before=_op.floor_divide(total_pad, two)
	pad_after=total_pad-pad_before
	# combine
	if"LOWER"inmode:
	pad=_op.concatenate(
	[_op.reshape(pad_after, [-1, 1]), _op.reshape(pad_before, [-1, 1])], axis=1
	)
	else:
	pad=_op.concatenate(
	[_op.reshape(pad_before, [-1, 1]), _op.reshape(pad_after, [-1, 1])], axis=1
	)
	# pad N and C with zeros
	pad=_op.concatenate([_op.const(np.zeros([2, 2], dtype="int64"), dtype="int64"), pad], axis=0)
	ifisinstance(pad_value, (float, int)):
	pad_value=_op.const(pad_value)
	return_op.nn.pad(data, fold_constant(pad), pad_value, pad_type)

It's fairly complicated, I'm totally cool if you want to punt on that until you need it.

[jiangjiajun]

Thanks, this will solve a big problem! But to avoid making this pull request more complicated to review, let's left this for the next pull request.

[AndrewZhaoLuo]

Some initial comments. I've gotten up to convert_fill_constant will review the rest later. Agree with mbrookhart. This is pretty big so we might need more eyes.

Can you send the most up to date english docs btw?

Also, does PaddlePaddle support operator versioning? How will you handle API changes in the future?

[AndrewZhaoLuo]

Can you use python/tvm/relay/frontend/common.py::infer_shape?

[jiangjiajun]

We have referred ONNX frontend, this function also comes from there https://github.com/apache/tvm/blob/main/python/tvm/relay/frontend/onnx.py#L1411
It's a little different from common::infer_shape

[AndrewZhaoLuo]

Can you use python/tvm/relay/frontend/common.py::infer_value?

[jiangjiajun]

Done. I just found there's try_infer_value in common.py, this function is removed.

[AndrewZhaoLuo]

In general Docstrings should be complete sentences. End with a period and capitalize the first letter.

E.g. "Calculate the paddings size."

Please fix the other docstrings

[jiangjiajun]

Done

[AndrewZhaoLuo]

Do you have a link to the english docs?

https://www.paddlepaddle.org.cn/documentation/docs/en/1.8/api/layers/argmax.html

Doesn't seem to have some of the attributes listed in the op

[jiangjiajun]

Now the latest version is 2.1, API documents: https://www.paddlepaddle.org.cn/documentation/docs/en/api/paddle/argmax_en.html#argmax

Follow the API definition code, we can find there's some attributes not list in the API's parameters
https://github.com/PaddlePaddle/Paddle/blob/release/2.1/python/paddle/tensor/search.py#L179

 attrs['keepdims'] = keepdim
attrs['axis'] = axis
attrs['flatten'] = flatten
attrs['dtype'] = var_dtype
helper.append_op(
type='arg_max', inputs={'X': x}, outputs={'Out': [out]}, attrs=attrs)
out.stop_gradient = True

[AndrewZhaoLuo]

A lot of the logic in argmin and argmax is similar. Refactor to combine the two.

[jiangjiajun]

Done

[AndrewZhaoLuo]

consider using _op.gather on the out_indices

[jiangjiajun]

Done

[AndrewZhaoLuo]

nit: out_indices

[jiangjiajun]

Done

[AndrewZhaoLuo]

You might want to note the semantics of paddle paddle's dot operator. Namely how it also operates on 2d-vectors (and hence why axis=[-1]).

I have not seen this elsewhere

[jiangjiajun]

paddle.dot : https://www.paddlepaddle.org.cn/documentation/docs/en/api/paddle/dot_en.html

It's similar with torch.dot, while torch.dot only supports 1D tensor.
In PaddlePaddle, inputs should be both 1D or 2D tensor. When it is 2d, the first dimension of this matrix is the batch dimension.

For clarify, I also put this explanation in code

[jiangjiajun]

Hi, @AndrewZhaoLuo@mbrookhart
Thanks for reviewing this PR.

1. PaddlePaddle now provides English document for API , there's no document describe operators, we are supporting this operators mostly by refer to its cpp code or test code, like crop_op.h or test_crop_tensor.py, but I think it's necessary to provide such documents, I'll try to push this within the PaddlePaddle team in the next quarter. For now, if you have any question about the operators, just comment in this pr, I'll try to make explanation here.
2. Like other framework, PaddlePaddle has needs to upgrade operators, this will add or delete some parameters for the operator, and also bring a new operator name, like squeeze2 or multiclass_nms3. Currently, we create a convert function for all the different versions of operator, but different versions of operator are both list in the _convert_map.

[jiangjiajun]

This PR is still too big I think considering most work here is unrelated to each other. Can you remove operators from this PR until you are down to ~+300 loc?

Just so you know, all ops above convert_fill_constant I have taken a look at so if you reduce this PR down to those changes only the review process can go a lot faster

Hi, @AndrewZhaoLuo
All the modifications under convert_fill_constant are removed .
Still lack of lots of pull requests to finish my work, I'll try to classify these pull requests to make reviewing faster

[AndrewZhaoLuo]

Coming together well. I think I am ok with merging this in the current state as part of a more experimental frontend, but would like another pair of eyes on this.

Still a few comments which will make this better:

1. Can you add more substantial test cases to your tests? E.g. different input shapes and those of different ranks at least for some of the relevant ops. I feel that some issues might come to light from this. Do not worry about PR size anymore

2. Please add type annotations to functions for this and future PRs. e.g.

def myFunc(a: int, b: List[string]) -> None:
...

[AndrewZhaoLuo]

I believe you might run into a similar issue as https://github.com/apache/tvm/blob/main/python/tvm/relay/frontend/onnx.py#L2257

[jiangjiajun]

PaddlePaddle's expand_as doesn't support multi-directional broadcasting, so this problem will not happen in PaddlePaddle frontend

[AndrewZhaoLuo]

Should describe padding size for what

[jiangjiajun]

Done

[jiangjiajun]

Coming together well. I think I am ok with merging this in the current state as part of a more experimental frontend, but would like another pair of eyes on this.

Still a few comments which will make this better:

1. Can you add more substantial test cases to your tests? E.g. different input shapes and those of different ranks at least for some of the relevant ops. I feel that some issues might come to light from this. Do not worry about PR size anymore
2. Please add type annotations to functions for this and future PRs. e.g.

def myFunc(a: int, b: List[string]) -> None: ...

• More cases have been added in tests, but only for the new operators in this pull request. I will send another pull request for the previous operators.
• Type annotation is a good code habit, but for the function like convert_dot(g : GraphProto, op : paddle.fluid.framework.operators, block: paddle.fluid.framework.Block), the type annotation will bring dependency of paddlepaddle for TVM, I noticed that all the frontends putting framework importing in from_xxx function to avoid strong dependency for TVM.

[AndrewZhaoLuo]

Shame about the typing, you can do forward references like:

def g(f:"paddle.paddleblahblah.blah"): -->:

But eh it's not the end of the world to be untyped since the rest of the frontends are like that. Just one comment about test case sizes.

We'll need another approver though. @mbrookhart ?

[AndrewZhaoLuo]

Please reduce the size of your test cases to something smaller e.g. less than 256 total elements (totally arbitrary, just as small as possible while still accomplishing the test)

[jiangjiajun]

Done. I guess the limit on the number of elements is to reduce the cost time of testing?

[AndrewZhaoLuo]

LGTM

[jiangjiajun]

@junrushao1994 Hi, could you help to merge this pull request?

[junrushao]

Thanks @AndrewZhaoLuo for the review! Thanks @jiangjiajun for the PR!

[jiangjiajun]

This pull request is part of #9102 hope this will bring some help to review

@AndrewZhaoLuo
Thanks for contributing to TVM! Please refer to guideline https://tvm.apache.org/docs/contribute/ for useful information and tips. After the pull request is submitted, please request code reviews from Reviewers by @ them in the pull request thread.

[mbrookhart]

This is really big, so it's hard to catch all of the edge cases in a review, but it looks okay, with the standard caveats that dynamic support might not be fully there yet. Not seeing anything I disapprove of, but I'd like to get more eyes on it before approving.

[mbrookhart]

Just as a heads up, I supported SAME padding in the ONNX frontend with dynamic shapes here:

tvm/python/tvm/relay/frontend/onnx.py

Lines 412 to 472 in d0c6ca5

	defautopad(
	data,
	strides,
	kernel_shape,
	dilations,
	ndim,
	pad_type="constant",
	deconv=False,
	mode="SAME_UPPER",
	pad_value=0.0,
	):
	"""
	Perform autopadding with dynamic input shapes
	"""
	# get attributes as constants
	strides=_op.const(np.array(strides), dtype="int64")
	dilated_kernel_shape=_op.const(
	np.array(
	[(kernel-1) *dilation+1forkernel, dilationinzip(kernel_shape, dilations)]
	),
	dtype="int64",
	)
	# get input shape
	shape=_op.strided_slice(shape_of(data, dtype="int64"), [2], [ndim])
	# set up integer constants
	zero=_op.const(0, dtype="int64")
	one=_op.const(1, dtype="int64")
	two=_op.const(2, dtype="int64")
	# Calculate total padding
	mod=_op.mod(shape, strides)
	left=_op.maximum(dilated_kernel_shape-strides, zero)
	right=_op.maximum(dilated_kernel_shape-mod, zero)
	total_pad=_op.where(_op.equal(mod, zero), left, right)
	ifdeconv:
	total_pad=_op.const(np.array(kernel_shape), dtype="int64") -one-total_pad
	# split total padding into before and after
	pad_before=_op.floor_divide(total_pad, two)
	pad_after=total_pad-pad_before
	# combine
	if"LOWER"inmode:
	pad=_op.concatenate(
	[_op.reshape(pad_after, [-1, 1]), _op.reshape(pad_before, [-1, 1])], axis=1
	)
	else:
	pad=_op.concatenate(
	[_op.reshape(pad_before, [-1, 1]), _op.reshape(pad_after, [-1, 1])], axis=1
	)
	# pad N and C with zeros
	pad=_op.concatenate([_op.const(np.zeros([2, 2], dtype="int64"), dtype="int64"), pad], axis=0)
	ifisinstance(pad_value, (float, int)):
	pad_value=_op.const(pad_value)
	return_op.nn.pad(data, fold_constant(pad), pad_value, pad_type)

It's fairly complicated, I'm totally cool if you want to punt on that until you need it.

[jiangjiajun]

Thanks, this will solve a big problem! But to avoid making this pull request more complicated to review, let's left this for the next pull request.

[AndrewZhaoLuo]

Some initial comments. I've gotten up to convert_fill_constant will review the rest later. Agree with mbrookhart. This is pretty big so we might need more eyes.

Can you send the most up to date english docs btw?

Also, does PaddlePaddle support operator versioning? How will you handle API changes in the future?

[AndrewZhaoLuo]

Can you use python/tvm/relay/frontend/common.py::infer_shape?

[jiangjiajun]

We have referred ONNX frontend, this function also comes from there https://github.com/apache/tvm/blob/main/python/tvm/relay/frontend/onnx.py#L1411
It's a little different from common::infer_shape

[AndrewZhaoLuo]

Can you use python/tvm/relay/frontend/common.py::infer_value?

[jiangjiajun]

Done. I just found there's try_infer_value in common.py, this function is removed.

[AndrewZhaoLuo]

In general Docstrings should be complete sentences. End with a period and capitalize the first letter.

E.g. "Calculate the paddings size."

Please fix the other docstrings

[jiangjiajun]

Done

[AndrewZhaoLuo]

Do you have a link to the english docs?

https://www.paddlepaddle.org.cn/documentation/docs/en/1.8/api/layers/argmax.html

Doesn't seem to have some of the attributes listed in the op

[jiangjiajun]

Now the latest version is 2.1, API documents: https://www.paddlepaddle.org.cn/documentation/docs/en/api/paddle/argmax_en.html#argmax

Follow the API definition code, we can find there's some attributes not list in the API's parameters
https://github.com/PaddlePaddle/Paddle/blob/release/2.1/python/paddle/tensor/search.py#L179

 attrs['keepdims'] = keepdim
attrs['axis'] = axis
attrs['flatten'] = flatten
attrs['dtype'] = var_dtype
helper.append_op(
type='arg_max', inputs={'X': x}, outputs={'Out': [out]}, attrs=attrs)
out.stop_gradient = True

[AndrewZhaoLuo]

A lot of the logic in argmin and argmax is similar. Refactor to combine the two.

[jiangjiajun]

Done

[AndrewZhaoLuo]

consider using _op.gather on the out_indices

[jiangjiajun]

Done

[AndrewZhaoLuo]

nit: out_indices

[jiangjiajun]

Done

[AndrewZhaoLuo]

You might want to note the semantics of paddle paddle's dot operator. Namely how it also operates on 2d-vectors (and hence why axis=[-1]).

I have not seen this elsewhere

[jiangjiajun]

paddle.dot : https://www.paddlepaddle.org.cn/documentation/docs/en/api/paddle/dot_en.html

It's similar with torch.dot, while torch.dot only supports 1D tensor.
In PaddlePaddle, inputs should be both 1D or 2D tensor. When it is 2d, the first dimension of this matrix is the batch dimension.

For clarify, I also put this explanation in code

[jiangjiajun]

Hi, @AndrewZhaoLuo@mbrookhart
Thanks for reviewing this PR.

1. PaddlePaddle now provides English document for API , there's no document describe operators, we are supporting this operators mostly by refer to its cpp code or test code, like crop_op.h or test_crop_tensor.py, but I think it's necessary to provide such documents, I'll try to push this within the PaddlePaddle team in the next quarter. For now, if you have any question about the operators, just comment in this pr, I'll try to make explanation here.
2. Like other framework, PaddlePaddle has needs to upgrade operators, this will add or delete some parameters for the operator, and also bring a new operator name, like squeeze2 or multiclass_nms3. Currently, we create a convert function for all the different versions of operator, but different versions of operator are both list in the _convert_map.

[jiangjiajun]

This PR is still too big I think considering most work here is unrelated to each other. Can you remove operators from this PR until you are down to ~+300 loc?

Just so you know, all ops above convert_fill_constant I have taken a look at so if you reduce this PR down to those changes only the review process can go a lot faster

Hi, @AndrewZhaoLuo
All the modifications under convert_fill_constant are removed .
Still lack of lots of pull requests to finish my work, I'll try to classify these pull requests to make reviewing faster

[AndrewZhaoLuo]

Coming together well. I think I am ok with merging this in the current state as part of a more experimental frontend, but would like another pair of eyes on this.

Still a few comments which will make this better:

1. Can you add more substantial test cases to your tests? E.g. different input shapes and those of different ranks at least for some of the relevant ops. I feel that some issues might come to light from this. Do not worry about PR size anymore

2. Please add type annotations to functions for this and future PRs. e.g.

def myFunc(a: int, b: List[string]) -> None:
...

[AndrewZhaoLuo]

I believe you might run into a similar issue as https://github.com/apache/tvm/blob/main/python/tvm/relay/frontend/onnx.py#L2257

[jiangjiajun]

PaddlePaddle's expand_as doesn't support multi-directional broadcasting, so this problem will not happen in PaddlePaddle frontend

[AndrewZhaoLuo]

Should describe padding size for what

[jiangjiajun]

Done

[jiangjiajun]

Coming together well. I think I am ok with merging this in the current state as part of a more experimental frontend, but would like another pair of eyes on this.

Still a few comments which will make this better:

1. Can you add more substantial test cases to your tests? E.g. different input shapes and those of different ranks at least for some of the relevant ops. I feel that some issues might come to light from this. Do not worry about PR size anymore
2. Please add type annotations to functions for this and future PRs. e.g.

def myFunc(a: int, b: List[string]) -> None: ...

• More cases have been added in tests, but only for the new operators in this pull request. I will send another pull request for the previous operators.
• Type annotation is a good code habit, but for the function like convert_dot(g : GraphProto, op : paddle.fluid.framework.operators, block: paddle.fluid.framework.Block), the type annotation will bring dependency of paddlepaddle for TVM, I noticed that all the frontends putting framework importing in from_xxx function to avoid strong dependency for TVM.

[AndrewZhaoLuo]

Shame about the typing, you can do forward references like:

def g(f:"paddle.paddleblahblah.blah"): -->:

But eh it's not the end of the world to be untyped since the rest of the frontends are like that. Just one comment about test case sizes.

We'll need another approver though. @mbrookhart ?

[AndrewZhaoLuo]

Please reduce the size of your test cases to something smaller e.g. less than 256 total elements (totally arbitrary, just as small as possible while still accomplishing the test)

[jiangjiajun]

Done. I guess the limit on the number of elements is to reduce the cost time of testing?

[AndrewZhaoLuo]

LGTM

[jiangjiajun]

@junrushao1994 Hi, could you help to merge this pull request?

[junrushao]

Thanks @AndrewZhaoLuo for the review! Thanks @jiangjiajun for the PR!

[jiangjiajun]

This pull request is part of #9102 hope this will bring some help to review

@AndrewZhaoLuo
Thanks for contributing to TVM! Please refer to guideline https://tvm.apache.org/docs/contribute/ for useful information and tips. After the pull request is submitted, please request code reviews from Reviewers by @ them in the pull request thread.

[mbrookhart]

This is really big, so it's hard to catch all of the edge cases in a review, but it looks okay, with the standard caveats that dynamic support might not be fully there yet. Not seeing anything I disapprove of, but I'd like to get more eyes on it before approving.

[mbrookhart]

Just as a heads up, I supported SAME padding in the ONNX frontend with dynamic shapes here:

tvm/python/tvm/relay/frontend/onnx.py

Lines 412 to 472 in d0c6ca5

	defautopad(
	data,
	strides,
	kernel_shape,
	dilations,
	ndim,
	pad_type="constant",
	deconv=False,
	mode="SAME_UPPER",
	pad_value=0.0,
	):
	"""
	Perform autopadding with dynamic input shapes
	"""
	# get attributes as constants
	strides=_op.const(np.array(strides), dtype="int64")
	dilated_kernel_shape=_op.const(
	np.array(
	[(kernel-1) *dilation+1forkernel, dilationinzip(kernel_shape, dilations)]
	),
	dtype="int64",
	)
	# get input shape
	shape=_op.strided_slice(shape_of(data, dtype="int64"), [2], [ndim])
	# set up integer constants
	zero=_op.const(0, dtype="int64")
	one=_op.const(1, dtype="int64")
	two=_op.const(2, dtype="int64")
	# Calculate total padding
	mod=_op.mod(shape, strides)
	left=_op.maximum(dilated_kernel_shape-strides, zero)
	right=_op.maximum(dilated_kernel_shape-mod, zero)
	total_pad=_op.where(_op.equal(mod, zero), left, right)
	ifdeconv:
	total_pad=_op.const(np.array(kernel_shape), dtype="int64") -one-total_pad
	# split total padding into before and after
	pad_before=_op.floor_divide(total_pad, two)
	pad_after=total_pad-pad_before
	# combine
	if"LOWER"inmode:
	pad=_op.concatenate(
	[_op.reshape(pad_after, [-1, 1]), _op.reshape(pad_before, [-1, 1])], axis=1
	)
	else:
	pad=_op.concatenate(
	[_op.reshape(pad_before, [-1, 1]), _op.reshape(pad_after, [-1, 1])], axis=1
	)
	# pad N and C with zeros
	pad=_op.concatenate([_op.const(np.zeros([2, 2], dtype="int64"), dtype="int64"), pad], axis=0)
	ifisinstance(pad_value, (float, int)):
	pad_value=_op.const(pad_value)
	return_op.nn.pad(data, fold_constant(pad), pad_value, pad_type)

It's fairly complicated, I'm totally cool if you want to punt on that until you need it.

[jiangjiajun]

Thanks, this will solve a big problem! But to avoid making this pull request more complicated to review, let's left this for the next pull request.

[AndrewZhaoLuo]

Some initial comments. I've gotten up to convert_fill_constant will review the rest later. Agree with mbrookhart. This is pretty big so we might need more eyes.

Can you send the most up to date english docs btw?

Also, does PaddlePaddle support operator versioning? How will you handle API changes in the future?

[AndrewZhaoLuo]

Can you use python/tvm/relay/frontend/common.py::infer_shape?

[jiangjiajun]

We have referred ONNX frontend, this function also comes from there https://github.com/apache/tvm/blob/main/python/tvm/relay/frontend/onnx.py#L1411
It's a little different from common::infer_shape

[AndrewZhaoLuo]

Can you use python/tvm/relay/frontend/common.py::infer_value?

[jiangjiajun]

Done. I just found there's try_infer_value in common.py, this function is removed.

[AndrewZhaoLuo]

In general Docstrings should be complete sentences. End with a period and capitalize the first letter.

E.g. "Calculate the paddings size."

Please fix the other docstrings

[jiangjiajun]

Done

[AndrewZhaoLuo]

Do you have a link to the english docs?

https://www.paddlepaddle.org.cn/documentation/docs/en/1.8/api/layers/argmax.html

Doesn't seem to have some of the attributes listed in the op

[jiangjiajun]

Now the latest version is 2.1, API documents: https://www.paddlepaddle.org.cn/documentation/docs/en/api/paddle/argmax_en.html#argmax

Follow the API definition code, we can find there's some attributes not list in the API's parameters
https://github.com/PaddlePaddle/Paddle/blob/release/2.1/python/paddle/tensor/search.py#L179

 attrs['keepdims'] = keepdim
attrs['axis'] = axis
attrs['flatten'] = flatten
attrs['dtype'] = var_dtype
helper.append_op(
type='arg_max', inputs={'X': x}, outputs={'Out': [out]}, attrs=attrs)
out.stop_gradient = True

[AndrewZhaoLuo]

A lot of the logic in argmin and argmax is similar. Refactor to combine the two.

[jiangjiajun]

Done

[AndrewZhaoLuo]

consider using _op.gather on the out_indices

[jiangjiajun]

Done

[AndrewZhaoLuo]

nit: out_indices

[jiangjiajun]

Done

[AndrewZhaoLuo]

You might want to note the semantics of paddle paddle's dot operator. Namely how it also operates on 2d-vectors (and hence why axis=[-1]).

I have not seen this elsewhere

[jiangjiajun]

paddle.dot : https://www.paddlepaddle.org.cn/documentation/docs/en/api/paddle/dot_en.html

It's similar with torch.dot, while torch.dot only supports 1D tensor.
In PaddlePaddle, inputs should be both 1D or 2D tensor. When it is 2d, the first dimension of this matrix is the batch dimension.

For clarify, I also put this explanation in code

[jiangjiajun]

Hi, @AndrewZhaoLuo@mbrookhart
Thanks for reviewing this PR.

1. PaddlePaddle now provides English document for API , there's no document describe operators, we are supporting this operators mostly by refer to its cpp code or test code, like crop_op.h or test_crop_tensor.py, but I think it's necessary to provide such documents, I'll try to push this within the PaddlePaddle team in the next quarter. For now, if you have any question about the operators, just comment in this pr, I'll try to make explanation here.
2. Like other framework, PaddlePaddle has needs to upgrade operators, this will add or delete some parameters for the operator, and also bring a new operator name, like squeeze2 or multiclass_nms3. Currently, we create a convert function for all the different versions of operator, but different versions of operator are both list in the _convert_map.

[jiangjiajun]

This PR is still too big I think considering most work here is unrelated to each other. Can you remove operators from this PR until you are down to ~+300 loc?

Just so you know, all ops above convert_fill_constant I have taken a look at so if you reduce this PR down to those changes only the review process can go a lot faster

Hi, @AndrewZhaoLuo
All the modifications under convert_fill_constant are removed .
Still lack of lots of pull requests to finish my work, I'll try to classify these pull requests to make reviewing faster

[AndrewZhaoLuo]

Coming together well. I think I am ok with merging this in the current state as part of a more experimental frontend, but would like another pair of eyes on this.

Still a few comments which will make this better:

1. Can you add more substantial test cases to your tests? E.g. different input shapes and those of different ranks at least for some of the relevant ops. I feel that some issues might come to light from this. Do not worry about PR size anymore

2. Please add type annotations to functions for this and future PRs. e.g.

def myFunc(a: int, b: List[string]) -> None:
...

[AndrewZhaoLuo]

I believe you might run into a similar issue as https://github.com/apache/tvm/blob/main/python/tvm/relay/frontend/onnx.py#L2257

[jiangjiajun]

PaddlePaddle's expand_as doesn't support multi-directional broadcasting, so this problem will not happen in PaddlePaddle frontend

[AndrewZhaoLuo]

Should describe padding size for what

[jiangjiajun]

Done

[jiangjiajun]

Coming together well. I think I am ok with merging this in the current state as part of a more experimental frontend, but would like another pair of eyes on this.

Still a few comments which will make this better:

1. Can you add more substantial test cases to your tests? E.g. different input shapes and those of different ranks at least for some of the relevant ops. I feel that some issues might come to light from this. Do not worry about PR size anymore
2. Please add type annotations to functions for this and future PRs. e.g.

def myFunc(a: int, b: List[string]) -> None: ...

• More cases have been added in tests, but only for the new operators in this pull request. I will send another pull request for the previous operators.
• Type annotation is a good code habit, but for the function like convert_dot(g : GraphProto, op : paddle.fluid.framework.operators, block: paddle.fluid.framework.Block), the type annotation will bring dependency of paddlepaddle for TVM, I noticed that all the frontends putting framework importing in from_xxx function to avoid strong dependency for TVM.

[AndrewZhaoLuo]

Shame about the typing, you can do forward references like:

def g(f:"paddle.paddleblahblah.blah"): -->:

But eh it's not the end of the world to be untyped since the rest of the frontends are like that. Just one comment about test case sizes.

We'll need another approver though. @mbrookhart ?

[AndrewZhaoLuo]

Please reduce the size of your test cases to something smaller e.g. less than 256 total elements (totally arbitrary, just as small as possible while still accomplishing the test)

[jiangjiajun]

Done. I guess the limit on the number of elements is to reduce the cost time of testing?

[AndrewZhaoLuo]

LGTM

[jiangjiajun]

@junrushao1994 Hi, could you help to merge this pull request?

[junrushao]

Thanks @AndrewZhaoLuo for the review! Thanks @jiangjiajun for the PR!

[jiangjiajun]

This pull request is part of #9102 hope this will bring some help to review

@AndrewZhaoLuo
Thanks for contributing to TVM! Please refer to guideline https://tvm.apache.org/docs/contribute/ for useful information and tips. After the pull request is submitted, please request code reviews from Reviewers by @ them in the pull request thread.

[mbrookhart]

This is really big, so it's hard to catch all of the edge cases in a review, but it looks okay, with the standard caveats that dynamic support might not be fully there yet. Not seeing anything I disapprove of, but I'd like to get more eyes on it before approving.

[mbrookhart]

Just as a heads up, I supported SAME padding in the ONNX frontend with dynamic shapes here:

tvm/python/tvm/relay/frontend/onnx.py

Lines 412 to 472 in d0c6ca5

	defautopad(
	data,
	strides,
	kernel_shape,
	dilations,
	ndim,
	pad_type="constant",
	deconv=False,
	mode="SAME_UPPER",
	pad_value=0.0,
	):
	"""
	Perform autopadding with dynamic input shapes
	"""
	# get attributes as constants
	strides=_op.const(np.array(strides), dtype="int64")
	dilated_kernel_shape=_op.const(
	np.array(
	[(kernel-1) *dilation+1forkernel, dilationinzip(kernel_shape, dilations)]
	),
	dtype="int64",
	)
	# get input shape
	shape=_op.strided_slice(shape_of(data, dtype="int64"), [2], [ndim])
	# set up integer constants
	zero=_op.const(0, dtype="int64")
	one=_op.const(1, dtype="int64")
	two=_op.const(2, dtype="int64")
	# Calculate total padding
	mod=_op.mod(shape, strides)
	left=_op.maximum(dilated_kernel_shape-strides, zero)
	right=_op.maximum(dilated_kernel_shape-mod, zero)
	total_pad=_op.where(_op.equal(mod, zero), left, right)
	ifdeconv:
	total_pad=_op.const(np.array(kernel_shape), dtype="int64") -one-total_pad
	# split total padding into before and after
	pad_before=_op.floor_divide(total_pad, two)
	pad_after=total_pad-pad_before
	# combine
	if"LOWER"inmode:
	pad=_op.concatenate(
	[_op.reshape(pad_after, [-1, 1]), _op.reshape(pad_before, [-1, 1])], axis=1
	)
	else:
	pad=_op.concatenate(
	[_op.reshape(pad_before, [-1, 1]), _op.reshape(pad_after, [-1, 1])], axis=1
	)
	# pad N and C with zeros
	pad=_op.concatenate([_op.const(np.zeros([2, 2], dtype="int64"), dtype="int64"), pad], axis=0)
	ifisinstance(pad_value, (float, int)):
	pad_value=_op.const(pad_value)
	return_op.nn.pad(data, fold_constant(pad), pad_value, pad_type)

It's fairly complicated, I'm totally cool if you want to punt on that until you need it.

[jiangjiajun]

Thanks, this will solve a big problem! But to avoid making this pull request more complicated to review, let's left this for the next pull request.

[AndrewZhaoLuo]

Some initial comments. I've gotten up to convert_fill_constant will review the rest later. Agree with mbrookhart. This is pretty big so we might need more eyes.

Can you send the most up to date english docs btw?

Also, does PaddlePaddle support operator versioning? How will you handle API changes in the future?

[AndrewZhaoLuo]

Can you use python/tvm/relay/frontend/common.py::infer_shape?

[jiangjiajun]

We have referred ONNX frontend, this function also comes from there https://github.com/apache/tvm/blob/main/python/tvm/relay/frontend/onnx.py#L1411
It's a little different from common::infer_shape

[AndrewZhaoLuo]

Can you use python/tvm/relay/frontend/common.py::infer_value?

[jiangjiajun]

Done. I just found there's try_infer_value in common.py, this function is removed.

[AndrewZhaoLuo]

In general Docstrings should be complete sentences. End with a period and capitalize the first letter.

E.g. "Calculate the paddings size."

Please fix the other docstrings

[jiangjiajun]

Done

[AndrewZhaoLuo]

Do you have a link to the english docs?

https://www.paddlepaddle.org.cn/documentation/docs/en/1.8/api/layers/argmax.html

Doesn't seem to have some of the attributes listed in the op

[jiangjiajun]

Now the latest version is 2.1, API documents: https://www.paddlepaddle.org.cn/documentation/docs/en/api/paddle/argmax_en.html#argmax

Follow the API definition code, we can find there's some attributes not list in the API's parameters
https://github.com/PaddlePaddle/Paddle/blob/release/2.1/python/paddle/tensor/search.py#L179

 attrs['keepdims'] = keepdim
attrs['axis'] = axis
attrs['flatten'] = flatten
attrs['dtype'] = var_dtype
helper.append_op(
type='arg_max', inputs={'X': x}, outputs={'Out': [out]}, attrs=attrs)
out.stop_gradient = True

[AndrewZhaoLuo]

A lot of the logic in argmin and argmax is similar. Refactor to combine the two.

[jiangjiajun]

Done

[AndrewZhaoLuo]

consider using _op.gather on the out_indices

[jiangjiajun]

Done

[AndrewZhaoLuo]

nit: out_indices

[jiangjiajun]

Done

[AndrewZhaoLuo]

You might want to note the semantics of paddle paddle's dot operator. Namely how it also operates on 2d-vectors (and hence why axis=[-1]).

I have not seen this elsewhere

[jiangjiajun]

paddle.dot : https://www.paddlepaddle.org.cn/documentation/docs/en/api/paddle/dot_en.html

It's similar with torch.dot, while torch.dot only supports 1D tensor.
In PaddlePaddle, inputs should be both 1D or 2D tensor. When it is 2d, the first dimension of this matrix is the batch dimension.

For clarify, I also put this explanation in code

[jiangjiajun]

Hi, @AndrewZhaoLuo@mbrookhart
Thanks for reviewing this PR.

1. PaddlePaddle now provides English document for API , there's no document describe operators, we are supporting this operators mostly by refer to its cpp code or test code, like crop_op.h or test_crop_tensor.py, but I think it's necessary to provide such documents, I'll try to push this within the PaddlePaddle team in the next quarter. For now, if you have any question about the operators, just comment in this pr, I'll try to make explanation here.
2. Like other framework, PaddlePaddle has needs to upgrade operators, this will add or delete some parameters for the operator, and also bring a new operator name, like squeeze2 or multiclass_nms3. Currently, we create a convert function for all the different versions of operator, but different versions of operator are both list in the _convert_map.

[jiangjiajun]

This PR is still too big I think considering most work here is unrelated to each other. Can you remove operators from this PR until you are down to ~+300 loc?

Just so you know, all ops above convert_fill_constant I have taken a look at so if you reduce this PR down to those changes only the review process can go a lot faster

Hi, @AndrewZhaoLuo
All the modifications under convert_fill_constant are removed .
Still lack of lots of pull requests to finish my work, I'll try to classify these pull requests to make reviewing faster

[AndrewZhaoLuo]

Coming together well. I think I am ok with merging this in the current state as part of a more experimental frontend, but would like another pair of eyes on this.

Still a few comments which will make this better:

1. Can you add more substantial test cases to your tests? E.g. different input shapes and those of different ranks at least for some of the relevant ops. I feel that some issues might come to light from this. Do not worry about PR size anymore

2. Please add type annotations to functions for this and future PRs. e.g.

def myFunc(a: int, b: List[string]) -> None:
...

[AndrewZhaoLuo]

I believe you might run into a similar issue as https://github.com/apache/tvm/blob/main/python/tvm/relay/frontend/onnx.py#L2257

[jiangjiajun]

PaddlePaddle's expand_as doesn't support multi-directional broadcasting, so this problem will not happen in PaddlePaddle frontend

[AndrewZhaoLuo]

Should describe padding size for what

[jiangjiajun]

Done

[jiangjiajun]

Coming together well. I think I am ok with merging this in the current state as part of a more experimental frontend, but would like another pair of eyes on this.

Still a few comments which will make this better:

1. Can you add more substantial test cases to your tests? E.g. different input shapes and those of different ranks at least for some of the relevant ops. I feel that some issues might come to light from this. Do not worry about PR size anymore
2. Please add type annotations to functions for this and future PRs. e.g.

def myFunc(a: int, b: List[string]) -> None: ...

• More cases have been added in tests, but only for the new operators in this pull request. I will send another pull request for the previous operators.
• Type annotation is a good code habit, but for the function like convert_dot(g : GraphProto, op : paddle.fluid.framework.operators, block: paddle.fluid.framework.Block), the type annotation will bring dependency of paddlepaddle for TVM, I noticed that all the frontends putting framework importing in from_xxx function to avoid strong dependency for TVM.

[AndrewZhaoLuo]

Shame about the typing, you can do forward references like:

def g(f:"paddle.paddleblahblah.blah"): -->:

But eh it's not the end of the world to be untyped since the rest of the frontends are like that. Just one comment about test case sizes.

We'll need another approver though. @mbrookhart ?

[AndrewZhaoLuo]

Please reduce the size of your test cases to something smaller e.g. less than 256 total elements (totally arbitrary, just as small as possible while still accomplishing the test)

[jiangjiajun]

Done. I guess the limit on the number of elements is to reduce the cost time of testing?

[AndrewZhaoLuo]

LGTM

[jiangjiajun]

@junrushao1994 Hi, could you help to merge this pull request?

[junrushao]

Thanks @AndrewZhaoLuo for the review! Thanks @jiangjiajun for the PR!

[jiangjiajun]

This pull request is part of #9102 hope this will bring some help to review

@AndrewZhaoLuo
Thanks for contributing to TVM! Please refer to guideline https://tvm.apache.org/docs/contribute/ for useful information and tips. After the pull request is submitted, please request code reviews from Reviewers by @ them in the pull request thread.

[mbrookhart]

This is really big, so it's hard to catch all of the edge cases in a review, but it looks okay, with the standard caveats that dynamic support might not be fully there yet. Not seeing anything I disapprove of, but I'd like to get more eyes on it before approving.

[mbrookhart]

Just as a heads up, I supported SAME padding in the ONNX frontend with dynamic shapes here:

tvm/python/tvm/relay/frontend/onnx.py

Lines 412 to 472 in d0c6ca5

	defautopad(
	data,
	strides,
	kernel_shape,
	dilations,
	ndim,
	pad_type="constant",
	deconv=False,
	mode="SAME_UPPER",
	pad_value=0.0,
	):
	"""
	Perform autopadding with dynamic input shapes
	"""
	# get attributes as constants
	strides=_op.const(np.array(strides), dtype="int64")
	dilated_kernel_shape=_op.const(
	np.array(
	[(kernel-1) *dilation+1forkernel, dilationinzip(kernel_shape, dilations)]
	),
	dtype="int64",
	)
	# get input shape
	shape=_op.strided_slice(shape_of(data, dtype="int64"), [2], [ndim])
	# set up integer constants
	zero=_op.const(0, dtype="int64")
	one=_op.const(1, dtype="int64")
	two=_op.const(2, dtype="int64")
	# Calculate total padding
	mod=_op.mod(shape, strides)
	left=_op.maximum(dilated_kernel_shape-strides, zero)
	right=_op.maximum(dilated_kernel_shape-mod, zero)
	total_pad=_op.where(_op.equal(mod, zero), left, right)
	ifdeconv:
	total_pad=_op.const(np.array(kernel_shape), dtype="int64") -one-total_pad
	# split total padding into before and after
	pad_before=_op.floor_divide(total_pad, two)
	pad_after=total_pad-pad_before
	# combine
	if"LOWER"inmode:
	pad=_op.concatenate(
	[_op.reshape(pad_after, [-1, 1]), _op.reshape(pad_before, [-1, 1])], axis=1
	)
	else:
	pad=_op.concatenate(
	[_op.reshape(pad_before, [-1, 1]), _op.reshape(pad_after, [-1, 1])], axis=1
	)
	# pad N and C with zeros
	pad=_op.concatenate([_op.const(np.zeros([2, 2], dtype="int64"), dtype="int64"), pad], axis=0)
	ifisinstance(pad_value, (float, int)):
	pad_value=_op.const(pad_value)
	return_op.nn.pad(data, fold_constant(pad), pad_value, pad_type)

It's fairly complicated, I'm totally cool if you want to punt on that until you need it.

[jiangjiajun]

Thanks, this will solve a big problem! But to avoid making this pull request more complicated to review, let's left this for the next pull request.

[AndrewZhaoLuo]

Some initial comments. I've gotten up to convert_fill_constant will review the rest later. Agree with mbrookhart. This is pretty big so we might need more eyes.

Can you send the most up to date english docs btw?

Also, does PaddlePaddle support operator versioning? How will you handle API changes in the future?

[AndrewZhaoLuo]

Can you use python/tvm/relay/frontend/common.py::infer_shape?

[jiangjiajun]

We have referred ONNX frontend, this function also comes from there https://github.com/apache/tvm/blob/main/python/tvm/relay/frontend/onnx.py#L1411
It's a little different from common::infer_shape

[AndrewZhaoLuo]

Can you use python/tvm/relay/frontend/common.py::infer_value?

[jiangjiajun]

Done. I just found there's try_infer_value in common.py, this function is removed.

[AndrewZhaoLuo]

In general Docstrings should be complete sentences. End with a period and capitalize the first letter.

E.g. "Calculate the paddings size."

Please fix the other docstrings

[jiangjiajun]

Done

[AndrewZhaoLuo]

Do you have a link to the english docs?

https://www.paddlepaddle.org.cn/documentation/docs/en/1.8/api/layers/argmax.html

Doesn't seem to have some of the attributes listed in the op

[jiangjiajun]

Now the latest version is 2.1, API documents: https://www.paddlepaddle.org.cn/documentation/docs/en/api/paddle/argmax_en.html#argmax

Follow the API definition code, we can find there's some attributes not list in the API's parameters
https://github.com/PaddlePaddle/Paddle/blob/release/2.1/python/paddle/tensor/search.py#L179

 attrs['keepdims'] = keepdim
attrs['axis'] = axis
attrs['flatten'] = flatten
attrs['dtype'] = var_dtype
helper.append_op(
type='arg_max', inputs={'X': x}, outputs={'Out': [out]}, attrs=attrs)
out.stop_gradient = True

[AndrewZhaoLuo]

A lot of the logic in argmin and argmax is similar. Refactor to combine the two.

[jiangjiajun]

Done

[AndrewZhaoLuo]

consider using _op.gather on the out_indices

[jiangjiajun]

Done

[AndrewZhaoLuo]

nit: out_indices

[jiangjiajun]

Done

[AndrewZhaoLuo]

You might want to note the semantics of paddle paddle's dot operator. Namely how it also operates on 2d-vectors (and hence why axis=[-1]).

I have not seen this elsewhere

[jiangjiajun]

paddle.dot : https://www.paddlepaddle.org.cn/documentation/docs/en/api/paddle/dot_en.html

It's similar with torch.dot, while torch.dot only supports 1D tensor.
In PaddlePaddle, inputs should be both 1D or 2D tensor. When it is 2d, the first dimension of this matrix is the batch dimension.

For clarify, I also put this explanation in code

[jiangjiajun]

Hi, @AndrewZhaoLuo@mbrookhart
Thanks for reviewing this PR.

1. PaddlePaddle now provides English document for API , there's no document describe operators, we are supporting this operators mostly by refer to its cpp code or test code, like crop_op.h or test_crop_tensor.py, but I think it's necessary to provide such documents, I'll try to push this within the PaddlePaddle team in the next quarter. For now, if you have any question about the operators, just comment in this pr, I'll try to make explanation here.
2. Like other framework, PaddlePaddle has needs to upgrade operators, this will add or delete some parameters for the operator, and also bring a new operator name, like squeeze2 or multiclass_nms3. Currently, we create a convert function for all the different versions of operator, but different versions of operator are both list in the _convert_map.

[jiangjiajun]

This PR is still too big I think considering most work here is unrelated to each other. Can you remove operators from this PR until you are down to ~+300 loc?

Just so you know, all ops above convert_fill_constant I have taken a look at so if you reduce this PR down to those changes only the review process can go a lot faster

Hi, @AndrewZhaoLuo
All the modifications under convert_fill_constant are removed .
Still lack of lots of pull requests to finish my work, I'll try to classify these pull requests to make reviewing faster

[AndrewZhaoLuo]

Coming together well. I think I am ok with merging this in the current state as part of a more experimental frontend, but would like another pair of eyes on this.

Still a few comments which will make this better:

1. Can you add more substantial test cases to your tests? E.g. different input shapes and those of different ranks at least for some of the relevant ops. I feel that some issues might come to light from this. Do not worry about PR size anymore

2. Please add type annotations to functions for this and future PRs. e.g.

def myFunc(a: int, b: List[string]) -> None:
...

[AndrewZhaoLuo]

I believe you might run into a similar issue as https://github.com/apache/tvm/blob/main/python/tvm/relay/frontend/onnx.py#L2257

[jiangjiajun]

PaddlePaddle's expand_as doesn't support multi-directional broadcasting, so this problem will not happen in PaddlePaddle frontend

[AndrewZhaoLuo]

Should describe padding size for what

[jiangjiajun]

Done

[jiangjiajun]

Coming together well. I think I am ok with merging this in the current state as part of a more experimental frontend, but would like another pair of eyes on this.

Still a few comments which will make this better:

1. Can you add more substantial test cases to your tests? E.g. different input shapes and those of different ranks at least for some of the relevant ops. I feel that some issues might come to light from this. Do not worry about PR size anymore
2. Please add type annotations to functions for this and future PRs. e.g.

def myFunc(a: int, b: List[string]) -> None: ...

• More cases have been added in tests, but only for the new operators in this pull request. I will send another pull request for the previous operators.
• Type annotation is a good code habit, but for the function like convert_dot(g : GraphProto, op : paddle.fluid.framework.operators, block: paddle.fluid.framework.Block), the type annotation will bring dependency of paddlepaddle for TVM, I noticed that all the frontends putting framework importing in from_xxx function to avoid strong dependency for TVM.

[AndrewZhaoLuo]

Shame about the typing, you can do forward references like:

def g(f:"paddle.paddleblahblah.blah"): -->:

But eh it's not the end of the world to be untyped since the rest of the frontends are like that. Just one comment about test case sizes.

We'll need another approver though. @mbrookhart ?

[AndrewZhaoLuo]

Please reduce the size of your test cases to something smaller e.g. less than 256 total elements (totally arbitrary, just as small as possible while still accomplishing the test)

[jiangjiajun]

Done. I guess the limit on the number of elements is to reduce the cost time of testing?

[AndrewZhaoLuo]

LGTM

[jiangjiajun]

@junrushao1994 Hi, could you help to merge this pull request?

[junrushao]

Thanks @AndrewZhaoLuo for the review! Thanks @jiangjiajun for the PR!

[jiangjiajun]

This pull request is part of #9102 hope this will bring some help to review

@AndrewZhaoLuo
Thanks for contributing to TVM! Please refer to guideline https://tvm.apache.org/docs/contribute/ for useful information and tips. After the pull request is submitted, please request code reviews from Reviewers by @ them in the pull request thread.

[mbrookhart]

This is really big, so it's hard to catch all of the edge cases in a review, but it looks okay, with the standard caveats that dynamic support might not be fully there yet. Not seeing anything I disapprove of, but I'd like to get more eyes on it before approving.

[mbrookhart]

Just as a heads up, I supported SAME padding in the ONNX frontend with dynamic shapes here:

tvm/python/tvm/relay/frontend/onnx.py

Lines 412 to 472 in d0c6ca5

	defautopad(
	data,
	strides,
	kernel_shape,
	dilations,
	ndim,
	pad_type="constant",
	deconv=False,
	mode="SAME_UPPER",
	pad_value=0.0,
	):
	"""
	Perform autopadding with dynamic input shapes
	"""
	# get attributes as constants
	strides=_op.const(np.array(strides), dtype="int64")
	dilated_kernel_shape=_op.const(
	np.array(
	[(kernel-1) *dilation+1forkernel, dilationinzip(kernel_shape, dilations)]
	),
	dtype="int64",
	)
	# get input shape
	shape=_op.strided_slice(shape_of(data, dtype="int64"), [2], [ndim])
	# set up integer constants
	zero=_op.const(0, dtype="int64")
	one=_op.const(1, dtype="int64")
	two=_op.const(2, dtype="int64")
	# Calculate total padding
	mod=_op.mod(shape, strides)
	left=_op.maximum(dilated_kernel_shape-strides, zero)
	right=_op.maximum(dilated_kernel_shape-mod, zero)
	total_pad=_op.where(_op.equal(mod, zero), left, right)
	ifdeconv:
	total_pad=_op.const(np.array(kernel_shape), dtype="int64") -one-total_pad
	# split total padding into before and after
	pad_before=_op.floor_divide(total_pad, two)
	pad_after=total_pad-pad_before
	# combine
	if"LOWER"inmode:
	pad=_op.concatenate(
	[_op.reshape(pad_after, [-1, 1]), _op.reshape(pad_before, [-1, 1])], axis=1
	)
	else:
	pad=_op.concatenate(
	[_op.reshape(pad_before, [-1, 1]), _op.reshape(pad_after, [-1, 1])], axis=1
	)
	# pad N and C with zeros
	pad=_op.concatenate([_op.const(np.zeros([2, 2], dtype="int64"), dtype="int64"), pad], axis=0)
	ifisinstance(pad_value, (float, int)):
	pad_value=_op.const(pad_value)
	return_op.nn.pad(data, fold_constant(pad), pad_value, pad_type)

It's fairly complicated, I'm totally cool if you want to punt on that until you need it.

[jiangjiajun]

Thanks, this will solve a big problem! But to avoid making this pull request more complicated to review, let's left this for the next pull request.

[AndrewZhaoLuo]

Some initial comments. I've gotten up to convert_fill_constant will review the rest later. Agree with mbrookhart. This is pretty big so we might need more eyes.

Can you send the most up to date english docs btw?

Also, does PaddlePaddle support operator versioning? How will you handle API changes in the future?

[AndrewZhaoLuo]

Can you use python/tvm/relay/frontend/common.py::infer_shape?

[jiangjiajun]

We have referred ONNX frontend, this function also comes from there https://github.com/apache/tvm/blob/main/python/tvm/relay/frontend/onnx.py#L1411
It's a little different from common::infer_shape

[AndrewZhaoLuo]

Can you use python/tvm/relay/frontend/common.py::infer_value?

[jiangjiajun]

Done. I just found there's try_infer_value in common.py, this function is removed.

[AndrewZhaoLuo]

In general Docstrings should be complete sentences. End with a period and capitalize the first letter.

E.g. "Calculate the paddings size."

Please fix the other docstrings

[jiangjiajun]

Done

[AndrewZhaoLuo]

Do you have a link to the english docs?

https://www.paddlepaddle.org.cn/documentation/docs/en/1.8/api/layers/argmax.html

Doesn't seem to have some of the attributes listed in the op

[jiangjiajun]

Now the latest version is 2.1, API documents: https://www.paddlepaddle.org.cn/documentation/docs/en/api/paddle/argmax_en.html#argmax

Follow the API definition code, we can find there's some attributes not list in the API's parameters
https://github.com/PaddlePaddle/Paddle/blob/release/2.1/python/paddle/tensor/search.py#L179

 attrs['keepdims'] = keepdim
attrs['axis'] = axis
attrs['flatten'] = flatten
attrs['dtype'] = var_dtype
helper.append_op(
type='arg_max', inputs={'X': x}, outputs={'Out': [out]}, attrs=attrs)
out.stop_gradient = True

[AndrewZhaoLuo]

A lot of the logic in argmin and argmax is similar. Refactor to combine the two.

[jiangjiajun]

Done

[AndrewZhaoLuo]

consider using _op.gather on the out_indices

[jiangjiajun]

Done

[AndrewZhaoLuo]

nit: out_indices

[jiangjiajun]

Done

[AndrewZhaoLuo]

You might want to note the semantics of paddle paddle's dot operator. Namely how it also operates on 2d-vectors (and hence why axis=[-1]).

I have not seen this elsewhere

[jiangjiajun]

paddle.dot : https://www.paddlepaddle.org.cn/documentation/docs/en/api/paddle/dot_en.html

It's similar with torch.dot, while torch.dot only supports 1D tensor.
In PaddlePaddle, inputs should be both 1D or 2D tensor. When it is 2d, the first dimension of this matrix is the batch dimension.

For clarify, I also put this explanation in code

[jiangjiajun]

Hi, @AndrewZhaoLuo@mbrookhart
Thanks for reviewing this PR.

1. PaddlePaddle now provides English document for API , there's no document describe operators, we are supporting this operators mostly by refer to its cpp code or test code, like crop_op.h or test_crop_tensor.py, but I think it's necessary to provide such documents, I'll try to push this within the PaddlePaddle team in the next quarter. For now, if you have any question about the operators, just comment in this pr, I'll try to make explanation here.
2. Like other framework, PaddlePaddle has needs to upgrade operators, this will add or delete some parameters for the operator, and also bring a new operator name, like squeeze2 or multiclass_nms3. Currently, we create a convert function for all the different versions of operator, but different versions of operator are both list in the _convert_map.

[jiangjiajun]

This PR is still too big I think considering most work here is unrelated to each other. Can you remove operators from this PR until you are down to ~+300 loc?

Just so you know, all ops above convert_fill_constant I have taken a look at so if you reduce this PR down to those changes only the review process can go a lot faster

Hi, @AndrewZhaoLuo
All the modifications under convert_fill_constant are removed .
Still lack of lots of pull requests to finish my work, I'll try to classify these pull requests to make reviewing faster

[AndrewZhaoLuo]

Coming together well. I think I am ok with merging this in the current state as part of a more experimental frontend, but would like another pair of eyes on this.

Still a few comments which will make this better:

1. Can you add more substantial test cases to your tests? E.g. different input shapes and those of different ranks at least for some of the relevant ops. I feel that some issues might come to light from this. Do not worry about PR size anymore

2. Please add type annotations to functions for this and future PRs. e.g.

def myFunc(a: int, b: List[string]) -> None:
...

[AndrewZhaoLuo]

I believe you might run into a similar issue as https://github.com/apache/tvm/blob/main/python/tvm/relay/frontend/onnx.py#L2257

[jiangjiajun]

PaddlePaddle's expand_as doesn't support multi-directional broadcasting, so this problem will not happen in PaddlePaddle frontend

[AndrewZhaoLuo]

Should describe padding size for what

[jiangjiajun]

Done

[jiangjiajun]

Coming together well. I think I am ok with merging this in the current state as part of a more experimental frontend, but would like another pair of eyes on this.

Still a few comments which will make this better:

1. Can you add more substantial test cases to your tests? E.g. different input shapes and those of different ranks at least for some of the relevant ops. I feel that some issues might come to light from this. Do not worry about PR size anymore
2. Please add type annotations to functions for this and future PRs. e.g.

def myFunc(a: int, b: List[string]) -> None: ...

• More cases have been added in tests, but only for the new operators in this pull request. I will send another pull request for the previous operators.
• Type annotation is a good code habit, but for the function like convert_dot(g : GraphProto, op : paddle.fluid.framework.operators, block: paddle.fluid.framework.Block), the type annotation will bring dependency of paddlepaddle for TVM, I noticed that all the frontends putting framework importing in from_xxx function to avoid strong dependency for TVM.

[AndrewZhaoLuo]

Shame about the typing, you can do forward references like:

def g(f:"paddle.paddleblahblah.blah"): -->:

But eh it's not the end of the world to be untyped since the rest of the frontends are like that. Just one comment about test case sizes.

We'll need another approver though. @mbrookhart ?

[AndrewZhaoLuo]

Please reduce the size of your test cases to something smaller e.g. less than 256 total elements (totally arbitrary, just as small as possible while still accomplishing the test)

[jiangjiajun]

Done. I guess the limit on the number of elements is to reduce the cost time of testing?

[AndrewZhaoLuo]

LGTM

[jiangjiajun]

@junrushao1994 Hi, could you help to merge this pull request?

[junrushao]

Thanks @AndrewZhaoLuo for the review! Thanks @jiangjiajun for the PR!

[jiangjiajun]

This pull request is part of #9102 hope this will bring some help to review

@AndrewZhaoLuo
Thanks for contributing to TVM! Please refer to guideline https://tvm.apache.org/docs/contribute/ for useful information and tips. After the pull request is submitted, please request code reviews from Reviewers by @ them in the pull request thread.

[mbrookhart]

This is really big, so it's hard to catch all of the edge cases in a review, but it looks okay, with the standard caveats that dynamic support might not be fully there yet. Not seeing anything I disapprove of, but I'd like to get more eyes on it before approving.

[mbrookhart]

Just as a heads up, I supported SAME padding in the ONNX frontend with dynamic shapes here:

tvm/python/tvm/relay/frontend/onnx.py

Lines 412 to 472 in d0c6ca5

	defautopad(
	data,
	strides,
	kernel_shape,
	dilations,
	ndim,
	pad_type="constant",
	deconv=False,
	mode="SAME_UPPER",
	pad_value=0.0,
	):
	"""
	Perform autopadding with dynamic input shapes
	"""
	# get attributes as constants
	strides=_op.const(np.array(strides), dtype="int64")
	dilated_kernel_shape=_op.const(
	np.array(
	[(kernel-1) *dilation+1forkernel, dilationinzip(kernel_shape, dilations)]
	),
	dtype="int64",
	)
	# get input shape
	shape=_op.strided_slice(shape_of(data, dtype="int64"), [2], [ndim])
	# set up integer constants
	zero=_op.const(0, dtype="int64")
	one=_op.const(1, dtype="int64")
	two=_op.const(2, dtype="int64")
	# Calculate total padding
	mod=_op.mod(shape, strides)
	left=_op.maximum(dilated_kernel_shape-strides, zero)
	right=_op.maximum(dilated_kernel_shape-mod, zero)
	total_pad=_op.where(_op.equal(mod, zero), left, right)
	ifdeconv:
	total_pad=_op.const(np.array(kernel_shape), dtype="int64") -one-total_pad
	# split total padding into before and after
	pad_before=_op.floor_divide(total_pad, two)
	pad_after=total_pad-pad_before
	# combine
	if"LOWER"inmode:
	pad=_op.concatenate(
	[_op.reshape(pad_after, [-1, 1]), _op.reshape(pad_before, [-1, 1])], axis=1
	)
	else:
	pad=_op.concatenate(
	[_op.reshape(pad_before, [-1, 1]), _op.reshape(pad_after, [-1, 1])], axis=1
	)
	# pad N and C with zeros
	pad=_op.concatenate([_op.const(np.zeros([2, 2], dtype="int64"), dtype="int64"), pad], axis=0)
	ifisinstance(pad_value, (float, int)):
	pad_value=_op.const(pad_value)
	return_op.nn.pad(data, fold_constant(pad), pad_value, pad_type)

It's fairly complicated, I'm totally cool if you want to punt on that until you need it.

[jiangjiajun]

Thanks, this will solve a big problem! But to avoid making this pull request more complicated to review, let's left this for the next pull request.

[AndrewZhaoLuo]

Some initial comments. I've gotten up to convert_fill_constant will review the rest later. Agree with mbrookhart. This is pretty big so we might need more eyes.

Can you send the most up to date english docs btw?

Also, does PaddlePaddle support operator versioning? How will you handle API changes in the future?

[AndrewZhaoLuo]

Can you use python/tvm/relay/frontend/common.py::infer_shape?

[jiangjiajun]

We have referred ONNX frontend, this function also comes from there https://github.com/apache/tvm/blob/main/python/tvm/relay/frontend/onnx.py#L1411
It's a little different from common::infer_shape

[AndrewZhaoLuo]

Can you use python/tvm/relay/frontend/common.py::infer_value?

[jiangjiajun]

Done. I just found there's try_infer_value in common.py, this function is removed.

[AndrewZhaoLuo]

In general Docstrings should be complete sentences. End with a period and capitalize the first letter.

E.g. "Calculate the paddings size."

Please fix the other docstrings

[jiangjiajun]

Done

[AndrewZhaoLuo]

Do you have a link to the english docs?

https://www.paddlepaddle.org.cn/documentation/docs/en/1.8/api/layers/argmax.html

Doesn't seem to have some of the attributes listed in the op

[jiangjiajun]

Now the latest version is 2.1, API documents: https://www.paddlepaddle.org.cn/documentation/docs/en/api/paddle/argmax_en.html#argmax

Follow the API definition code, we can find there's some attributes not list in the API's parameters
https://github.com/PaddlePaddle/Paddle/blob/release/2.1/python/paddle/tensor/search.py#L179

 attrs['keepdims'] = keepdim
attrs['axis'] = axis
attrs['flatten'] = flatten
attrs['dtype'] = var_dtype
helper.append_op(
type='arg_max', inputs={'X': x}, outputs={'Out': [out]}, attrs=attrs)
out.stop_gradient = True

[AndrewZhaoLuo]

A lot of the logic in argmin and argmax is similar. Refactor to combine the two.

[jiangjiajun]

Done

[AndrewZhaoLuo]

consider using _op.gather on the out_indices

[jiangjiajun]

Done

[AndrewZhaoLuo]

nit: out_indices

[jiangjiajun]

Done

[AndrewZhaoLuo]

You might want to note the semantics of paddle paddle's dot operator. Namely how it also operates on 2d-vectors (and hence why axis=[-1]).

I have not seen this elsewhere

[jiangjiajun]

paddle.dot : https://www.paddlepaddle.org.cn/documentation/docs/en/api/paddle/dot_en.html

It's similar with torch.dot, while torch.dot only supports 1D tensor.
In PaddlePaddle, inputs should be both 1D or 2D tensor. When it is 2d, the first dimension of this matrix is the batch dimension.

For clarify, I also put this explanation in code

[jiangjiajun]

Hi, @AndrewZhaoLuo@mbrookhart
Thanks for reviewing this PR.

1. PaddlePaddle now provides English document for API , there's no document describe operators, we are supporting this operators mostly by refer to its cpp code or test code, like crop_op.h or test_crop_tensor.py, but I think it's necessary to provide such documents, I'll try to push this within the PaddlePaddle team in the next quarter. For now, if you have any question about the operators, just comment in this pr, I'll try to make explanation here.
2. Like other framework, PaddlePaddle has needs to upgrade operators, this will add or delete some parameters for the operator, and also bring a new operator name, like squeeze2 or multiclass_nms3. Currently, we create a convert function for all the different versions of operator, but different versions of operator are both list in the _convert_map.

[jiangjiajun]

This PR is still too big I think considering most work here is unrelated to each other. Can you remove operators from this PR until you are down to ~+300 loc?

Just so you know, all ops above convert_fill_constant I have taken a look at so if you reduce this PR down to those changes only the review process can go a lot faster

Hi, @AndrewZhaoLuo
All the modifications under convert_fill_constant are removed .
Still lack of lots of pull requests to finish my work, I'll try to classify these pull requests to make reviewing faster

[AndrewZhaoLuo]

Coming together well. I think I am ok with merging this in the current state as part of a more experimental frontend, but would like another pair of eyes on this.

Still a few comments which will make this better:

1. Can you add more substantial test cases to your tests? E.g. different input shapes and those of different ranks at least for some of the relevant ops. I feel that some issues might come to light from this. Do not worry about PR size anymore

2. Please add type annotations to functions for this and future PRs. e.g.

def myFunc(a: int, b: List[string]) -> None:
...

[AndrewZhaoLuo]

I believe you might run into a similar issue as https://github.com/apache/tvm/blob/main/python/tvm/relay/frontend/onnx.py#L2257

[jiangjiajun]

PaddlePaddle's expand_as doesn't support multi-directional broadcasting, so this problem will not happen in PaddlePaddle frontend

[AndrewZhaoLuo]

Should describe padding size for what

[jiangjiajun]

Done

[jiangjiajun]

Coming together well. I think I am ok with merging this in the current state as part of a more experimental frontend, but would like another pair of eyes on this.

Still a few comments which will make this better:

1. Can you add more substantial test cases to your tests? E.g. different input shapes and those of different ranks at least for some of the relevant ops. I feel that some issues might come to light from this. Do not worry about PR size anymore
2. Please add type annotations to functions for this and future PRs. e.g.

def myFunc(a: int, b: List[string]) -> None: ...

• More cases have been added in tests, but only for the new operators in this pull request. I will send another pull request for the previous operators.
• Type annotation is a good code habit, but for the function like convert_dot(g : GraphProto, op : paddle.fluid.framework.operators, block: paddle.fluid.framework.Block), the type annotation will bring dependency of paddlepaddle for TVM, I noticed that all the frontends putting framework importing in from_xxx function to avoid strong dependency for TVM.

[AndrewZhaoLuo]

Shame about the typing, you can do forward references like:

def g(f:"paddle.paddleblahblah.blah"): -->:

But eh it's not the end of the world to be untyped since the rest of the frontends are like that. Just one comment about test case sizes.

We'll need another approver though. @mbrookhart ?

[AndrewZhaoLuo]

Please reduce the size of your test cases to something smaller e.g. less than 256 total elements (totally arbitrary, just as small as possible while still accomplishing the test)

[jiangjiajun]

Done. I guess the limit on the number of elements is to reduce the cost time of testing?

[AndrewZhaoLuo]

LGTM

[jiangjiajun]

@junrushao1994 Hi, could you help to merge this pull request?

[junrushao]

Thanks @AndrewZhaoLuo for the review! Thanks @jiangjiajun for the PR!