Skip to content
This repository has been archived by the owner on Aug 15, 2019. It is now read-only.

Add tf.spaceToBatchND #1176

Merged
merged 4 commits into from
Jul 21, 2018
Merged

Add tf.spaceToBatchND #1176

merged 4 commits into from
Jul 21, 2018

Conversation

dikatok
Copy link
Contributor

@dikatok dikatok commented Jul 20, 2018

Description

Adds batchToSpaceND to the API tensorflow/tfjs#161.


For repository owners only:

Please remember to apply all applicable tags to your pull request.
Tags: FEATURE, BREAKING, BUG, PERF, DEV, DOC, SECURITY

For more info see: https://github.com/tensorflow/tfjs/blob/master/DEVELOPMENT.md


This change is Reviewable

@dikatok
Copy link
Contributor Author

dikatok commented Jul 20, 2018

@rodrigob the only interaction I can think of is using them to implement atrous convolution, or do you have any suggestions?

Copy link
Contributor

@dsmilkov dsmilkov left a comment

Choose a reason for hiding this comment

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

This is really nice and it's almost there! Left 3 small comments. Ping me when you address it and I'll be happy to merge!

Reviewed 7 of 7 files at r1.
Reviewable status: 0 of 1 approvals obtained (waiting on @dikatok)


src/kernels/backend.ts, line 205 at r1 (raw file):

  spaceToBatchND<T extends Tensor>(
      x: T, blockShape: number[], crops: number[][]): T;

rename crops to paddings for consistency


src/ops/array_ops.ts, line 810 at r1 (raw file):

}

function spaceToBatchND_<T extends Tensor>(

Add a jsdoc to this method similar to the one for batchToSpaceND above. Also make sure to include the @doc {heading: 'Tensors', subheading: 'Transformations'} tag so we can surface the method in our website api page: https://js.tensorflow.org/api/latest


src/ops/array_ops_test.ts, line 3206 at r1 (raw file):

  });

  it('tensor4d, input shape=[1, 4, 4, 1], blockShape=[2, 2]', () => {

add you add one more test with input shape [2, 6, 6, 1] and block shape [2, 2] and verify its output is [8, 3, 3, 1] (used 6 with 2 to break the symmetry: 6 = 2 * 3, since 4 = 2 * 2 makes the block size equal the leftover spacial size)

@rodrigob
Copy link

@dikatok I have not studied the operations definition in detail, but is there not an expected symmetry ?

Something like
assert A == spaceToBatchND( batchToSpaceND( A ) ) ?

@dikatok
Copy link
Contributor Author

dikatok commented Jul 20, 2018

@rodrigob hmmm ok I will add some symmetry tests

@dsmilkov
Copy link
Contributor

@rodrigob That's a great suggestion

@dikatok
Copy link
Contributor Author

dikatok commented Jul 21, 2018

whoops, forgot to ping you @dsmilkov

Copy link
Contributor

@dsmilkov dsmilkov left a comment

Choose a reason for hiding this comment

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

Really nice work! Thank you!

Reviewed 3 of 3 files at r3.
Reviewable status: 0 of 1 approvals obtained

@dsmilkov dsmilkov changed the title [WIP] Add spaceToBatchND op Add tf.spaceToBatchND Jul 21, 2018
@dsmilkov dsmilkov merged commit c14e690 into tensorflow:master Jul 21, 2018
@dikatok dikatok deleted the spaceToBatchND branch August 22, 2018 10:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants