Skip to content
This repository was archived by the owner on Jul 1, 2023. It is now read-only.

Conversation

Shashi456
Copy link
Contributor

Added layer and respective vjps, the build also passes locally.
I'll write the tests in a later PR. It's pretty difficult to write tests for conv layers.

@Shashi456
Copy link
Contributor Author

Shashi456 commented May 21, 2019

Tested the build on nightly btw, If there are any review changes I will update them.
@dan-zheng @rxwei requesting review.

Copy link
Member

@dan-zheng dan-zheng left a comment

Choose a reason for hiding this comment

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

Thank you! Could you please add a test to Tests/DeepLearningTests/LayerTests.swift?

@Shashi456
Copy link
Contributor Author

@dan-zheng, is it okay if I add tests in a later PR? It's a little difficult to think a test case for 3-d convolution.

@dan-zheng
Copy link
Member

@dan-zheng, is it okay if I add tests in a later PR? It's a little difficult to think a test case for 3-d convolution.

Actually, could you please add a test in this PR? For PRs that add new functionality, we try to add tests for the new functionality. It helps couple functionality and tests, and ensures that new functionality is correct.

@Shashi456
Copy link
Contributor Author

Shashi456 commented May 22, 2019

@dan-zheng on some other note, could you tell me about the V2 versions in the raw api. Like convolved3dbackpropinput and filter both have a v2 version as well, are v2 versions faster?

@dan-zheng
Copy link
Member

@dan-zheng on some other note, could you tell me about the V2 versions in the raw api. Like convolved3dbackpropinput and filter both have a v2 version as well, are v2 versions faster?

Yes, please use V2 raw ops when they exist. V2 ops have fixed semantics or performance improvements.

@Shashi456
Copy link
Contributor Author

Shashi456 commented May 22, 2019

@dan-zheng Updated the functions to V2. In this case, I think it was fixed semantics. The build also passes now. I am still thinking of a proper test for conv3d, so I will do that in a while.
If you have suggestions please do give them to me :).

@dan-zheng
Copy link
Member

dan-zheng commented May 22, 2019

@dan-zheng Update the functions to V2. In this case, I think it was fixed semantics. The build also passes now. I am still thinking of a proper test for conv3d, so I will do that in a while.
If you have suggestions please do give them to me :).

One reliable way to write a test program using Python TensorFlow as a reference, then port the test to Swift.

Here's an example:

from functools import reduce
import operator
import tensorflow as tf

def product(iterable):
    return reduce(operator.mul, iterable, 1)

# Returns a tensor with increasing scalar values starting from zero,
# with the given shape.
def iota(shape):
    x = tf.range(0, product(shape), dtype=tf.float32)
    return tf.reshape(x, shape)

input_shape = [2, 1, 2, 1, 3]
filter_shape = [2, 2, 2, 3, 5]
strides = (1, 3, 2, 2, 1)
input = iota(input_shape)
filter = iota(filter_shape)
conv3d = tf.nn.conv3d(input, filter, strides=strides, padding='SAME')

with tf.Session() as session:
  result = session.run(conv3d)
  print(result.shape)
  # (2, 1, 1, 1, 5)
  print(result)
  # [[[[[ 455.  470.  485.  500.  515.]]]]
  # 
  # 
  # 
  #  [[[[1175. 1226. 1277. 1328. 1379.]]]]]

# Write a Swift for TensorFlow test checking result shape/values!

@Shashi456
Copy link
Contributor Author

Shashi456 commented May 22, 2019

@dan-zheng done. It passes locally.

@Shashi456
Copy link
Contributor Author

@dan-zheng requesting a final review. Thanks for your time.

- Change `testConv3D` to use a non-trivial bias.
- Remove calls to `round` when calling `XCTAssertEqual`.
- Fix `testAvgPool3D`.
@dan-zheng
Copy link
Member

Thanks @Shashi456!

I pushed a commit fixing tests: cbeea35

A few notes:

  • Please do not use round when calling XCTAssertEqual, as it significantly changes the values of tensors and is not appropriate for checking equality.

  • In testConv3D, I changed bias to be non-zero to make the test less trivial, and updated the expected output.

  • Tests didn't pass for me due to a crash in testAvgPool3D, so I'm curious how tests passed locally for you. After adding your test, please run swift test locally and make sure things pass!

    $ swift test
    Test Suite 'All tests' started at 2019-05-22 09:36:42.821
    Test Suite 'DeepLearningPackageTests.xctest' started at 2019-05-22 09:36:42.821
    Test Suite 'ContextTests' started at 2019-05-22 09:36:42.821
    Test Case '-[DeepLearningTests.ContextTests testDropout]' started.
    Test Case '-[DeepLearningTests.ContextTests testDropout]' passed (0.076 seconds).
    Test Case '-[DeepLearningTests.ContextTests testMultithreadedDropout]' started.
    Test Case '-[DeepLearningTests.ContextTests testMultithreadedDropout]' passed (0.001 seconds).
    Test Suite 'ContextTests' passed at 2019-05-22 09:36:42.898.
    	 Executed 2 tests, with 0 failures (0 unexpected) in 0.077 (0.077) seconds
    Test Suite 'LayerTests' started at 2019-05-22 09:36:42.899
    Test Case '-[DeepLearningTests.LayerTests testAvgPool1D]' started.
    Test Case '-[DeepLearningTests.LayerTests testAvgPool1D]' passed (0.001 seconds).
    Test Case '-[DeepLearningTests.LayerTests testAvgPool2D]' started.
    Test Case '-[DeepLearningTests.LayerTests testAvgPool2D]' passed (0.000 seconds).
    Test Case '-[DeepLearningTests.LayerTests testAvgPool3D]' started.
    Precondition failed: file /Users/danielzheng/swift-merge/swift/stdlib/public/TensorFlow/Tensor.swift, line 143
    Exited with signal code 4
    

Copy link
Member

@dan-zheng dan-zheng left a comment

Choose a reason for hiding this comment

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

Thank you @Shashi456, great work! 🙂

@dan-zheng dan-zheng merged commit e6036b4 into tensorflow:master May 22, 2019
@Shashi456
Copy link
Contributor Author

Shashi456 commented May 22, 2019

@dan-zheng I'm actually curious about how your avgpool3d test cases failed, In my case they didn't. The only error i've been getting are the linker errors.

@dan-zheng
Copy link
Member

@dan-zheng I'm actually curious about how your avgpool3d test cases failed, In my case they didn't. The only error i've been getting are the linker errors.

Hmm, that is strange. ¯_(ツ)_/¯
I'll look into the linker errors today.

@Shashi456
Copy link
Contributor Author

@dan-zheng one last thing :P, We are done with Conv 1D, 2D, 3D and all pooling layers. We are done with the basic recurrent cells as well. Any specific layers higher up in the priority?

@dan-zheng
Copy link
Member

How about adding a test for Conv2D?
Other than that, adding layers is up to you! You can pick what seems interesting. 🙂

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants