Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Relay] Add space_to_batch_nd and batch_to_space_nd operators #6477

Merged
merged 6 commits into from
Nov 17, 2020

Conversation

BhushanIMG
Copy link
Contributor

Hi,

I am Bhushan from Imagination Technologies.

This PR adds space_to_batch_nd and batch_to_space_nd operators to Relay and TOPI.
I have added the tests for these operators and Tensorflow frontend is also modified to use these operators.

Please review @tqchen @masahi @Huyuwei and let me know your comments.

@u99127
Copy link
Contributor

u99127 commented Sep 17, 2020

A very quick review : I think as part of this we should update the tflite frontend at the same time.

@zhiics
Copy link
Member

zhiics commented Sep 18, 2020

cc @kevinthesun @yongwww @srkreddy1238, please help review. Thanks.



def space_to_batch_nd(data, block_shape, paddings):
r"""Divide spatial dimensions of the data into a grid of blocks and interleave them into batch dim.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ci failed due to too long of this line

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have corrected this.

@BhushanIMG
Copy link
Contributor Author

A very quick review : I think as part of this we should update the tflite frontend at the same time.

@u99127 Modified tflite frontend to use batch_to_space_nd and space_to_batch_nd operators.

@tqchen
Copy link
Member

tqchen commented Oct 7, 2020

also cc @masahi @FrozenGene please help manage this PR

}

// pad the input with paddings provided
padded_t = pad(data, pad_before_int32, pad_after_int32, make_const(DataType::Int(32), 0));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about adding one pad_value parameter whose default value is 0 for space_to_batch? I imagine if we use it in the quantized model, its pad value will be zero_point, not 0.https://github.com/apache/incubator-tvm/blob/master/python/tvm/relay/frontend/tflite.py#L2423

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@FrozenGene Added pad_value parameter to space_to_batch_nd operator with default value 0.

tvm::Array<Integer> axis;
tvm::Array<PrimExpr> o_shape;

size_t M = block_shape.size();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we shouldn't use capital letter here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrected.

Comment on lines 577 to 578
size_t M = block_shape.size();
size_t N = in_shape.size();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

corrected.

import numpy as np


def space_to_batch_nd_python(data, block_shape, pad_before, pad_after):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add pad_value = 0 like previous comment said

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@tqchen tqchen changed the base branch from master to main October 11, 2020 18:20
@BhushanIMG
Copy link
Contributor Author

@FrozenGene Could you please spare some time to review this.

@FrozenGene FrozenGene merged commit 7e90e7d into apache:main Nov 17, 2020
@FrozenGene
Copy link
Member

thanks @BhushanIMG

trevor-m pushed a commit to trevor-m/tvm that referenced this pull request Dec 2, 2020
…#6477)

* [Relay] Add space_to_batch_nd and batch_to_space_nd operators

* Correct python-format errors

* correct lint errors

* tflite frontend to use batch_to_space and space_to_batch operators

* Add new pad_value parameter with default value is 0 for space_to_batch_nd and correct variable names

* Fix cppdocs - add documentation for pad_value
trevor-m pushed a commit to trevor-m/tvm that referenced this pull request Dec 4, 2020
…#6477)

* [Relay] Add space_to_batch_nd and batch_to_space_nd operators

* Correct python-format errors

* correct lint errors

* tflite frontend to use batch_to_space and space_to_batch operators

* Add new pad_value parameter with default value is 0 for space_to_batch_nd and correct variable names

* Fix cppdocs - add documentation for pad_value
trevor-m pushed a commit to neo-ai/tvm that referenced this pull request Dec 4, 2020
…#6477)

* [Relay] Add space_to_batch_nd and batch_to_space_nd operators

* Correct python-format errors

* correct lint errors

* tflite frontend to use batch_to_space and space_to_batch operators

* Add new pad_value parameter with default value is 0 for space_to_batch_nd and correct variable names

* Fix cppdocs - add documentation for pad_value
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants