Add Avgpool3d and Maxpool3d Layers & Tests

[Shashi456]

Before adding tests i wanted to ask, @rxwei can we overload init? Because what we currently support in the max pooling and avg pooling layers is non-square windows, for square windows someone would have to write the layer as
MaxPool3d(poolsize:(3,3,3), stride:(2,2,2)

Is there any way to work around that?

[saeta]

@Shashi456 yes you can overload init, and you can have one init delegate to another init as well. Having convenience initializers to simplify common cases is a good idea.

[Shashi456]

Can you explain what you mean by delegate? @saeta

[saeta]

Great question! In the Dense layer, we have one initializer call a different initializer to reduce duplication of code: https://github.com/tensorflow/swift-apis/blob/master/Sources/DeepLearning/Layer.swift#L245 Does that make sense?

[Shashi456]

Oh that's pretty cool. So you mean in this case, something like:

public init(poolSize: (Int), strides: (Int), padding: Padding = .valid) {
        self.poolsize = poolsize
        self.stride = stride
        self.padding = padding
    }
{ self.init(poolsize : (poolsize,poolsize,poolsize) ....}

In similar fashion?

[saeta]

Yes, and you can even refactor the 2 initializers you currently have in the PR into one delegating (calling) the other. :-)

[Shashi456]

You mean initialization delegation is possible across functions?

[dan-zheng]

Could you please add tests to Tests/DeepLearningTests/LayerTests.swift?
Let's try to maintain tests for every layer!

[rxwei]

Thanks! Please also address comments above (add tests).

[Shashi456]

@rxwei so the things i have to incorporate is :

• Tests for average and max pooling layers
• Add another init which reference to the original init

would that be it?

Also i had a doubt, to init function in the case of dense was not written in the same function, so would i have write public extension MaxPooling3D in this case as well?

[Shashi456]

@dan-zheng @saeta @rxwei please review!

[dan-zheng]

Please be mindful of the following:

• Use 4-space indentation.
• Wrap { onto the previous line.
• Local variable bindings like let poolsize = poolSize are not meaningful, use the argument poolSize directly.

I hope those points make sense! Being mindful of them helps us merge your PR more quickly.

Please use the following:

public extension AvgPool3D {
    /// Creates an average pooling layer with the specified pooling window size and stride. All
    /// pooling sizes and strides are the same.
    init(poolSize: Int, strides: Int, padding: Padding = .valid) {
        self.init(poolSize: (poolSize, poolSize, poolSize),
                  strides: (strides, strides, strides),
                  padding: padding)
    }
}

[Shashi456]

Yes, it does. I will pay heed from the next time, Sorry!

[Shashi456]

Also @dan-zheng can you think of any other variations of inits and should i extend an init for 2d max and average pooling as well?

[dan-zheng]

Note that this API is not comprehensive: both pooling window sizes and strides can either be specified as "multiple integers" or "one integer, implicitly duplicated".

In other words, all of the following may be useful:

• init(poolSize: (Int, Int, Int), strides: (Int, Int, Int), padding: Padding)
• init(poolSize: Int, strides: Int, padding: Padding)
• init(poolSize: (Int, Int, Int), strides: Int, padding: Padding)
• init(poolSize: Int, strides: (Int, Int, Int) padding: Padding)

Something to think about and visit after this PR.
Here's Keras (note "integer or tuple of two integers"):

[Shashi456]

While writing the code for this, i thought about exactly this case, but had no idea how to address it exactly. In python, It is easier to handle. I'm not very well versed with init overloading in swift

[dan-zheng]

Yes, it does. I will pay heed from the next time, Sorry!

Thanks a ton! That would be a huge help. 🙂

Also @dan-zheng can you think of any other variations of inits and should i extend an init for 2d max and average pooling as well?

I commented above about this.
To match Keras, we could have the following combinations:

• poolSize: (Int, Int, Int) or Int
• strides: (Int, Int, Int) or Int? (if nil, then use poolSize as strides)

I would recommend adding such initializers in a follow-up to this PR, to make API additions more incremental.

[Shashi456]

@dan-zheng thank you for being so patient with these reviews.

[Shashi456]

@rxwei can you review this once more?

[rxwei]

This should not have been merged because it does not compile. The self.init statement cannot appear on top level.

@Shashi456 Could you make sure everything compiles with the latest toolchain before you commit?

[rxwei]

I reverted it in #118. Feel free to start a new PR when you have a fix.

[Shashi456]

@rxwei when i just try to simply add a layer in colab, to get an idea of how the layer works. It often throws me this error, could you tell me how i can solve this ?

error: <Cell 8>:2:15: error: type 'MaxPool3D<Scalar>' does not conform to protocol 'Layer'
public struct MaxPool3D<Scalar: TensorFlowFloatingPoint>: Layer {
              ^

TensorFlow.Layer:2:20: note: protocol requires nested type 'Input'; do you want to add it?
    associatedtype Input : Differentiable
                   ^

TensorFlow.Layer:3:20: note: protocol requires nested type 'Output'; do you want to add it?
    associatedtype Output : Differentiable

[rxwei]

This is sometimes because of missing @differentiable on call(_:). Depends on the actual situation though. Error messages are not great and we will improve them over time.

[Shashi456]

Is there a temporary fix I could do @rxwei?

[rxwei]

Could you share a code example that I can use to reproduce your error?

[Shashi456]

@rxwei for example, if i use this code, I know this could also be because max is not differentiable, but this input output error is common with any layer i try to define.

@_fixed_layout
public struct GlobalMaxPooling3D<Scalar: TensorFlowFloatingPoint>: Layer {
    /// Creates a global max pooling layer.
    public init() {}

    /// Returns the output obtained from applying the layer to the given input.
    ///
    /// - Parameters:
    ///   - input: The input to the layer.
    ///   - context: The contextual information for the layer application, e.g. the current learning
    ///     phase.
    /// - Returns: The output.
    @differentiable
    public func applied(to input: Tensor<Scalar>, in _: Context) -> Tensor<Scalar> {
        return input.max(squeezingAxes: [1, 2, 3])
    }
}

[rxwei]

Colab is not yet updated with the latest APIs (v0.3), and that is causing your errors. You can follow the instructions here to install a new build to Colab.

[Shashi456]

Thanks a lot ! I will try and see if this works.

[rxwei]

Actually, it should be updated now if you restart Colab.

[Shashi456]

@rxwei the error majorly seems to be :

error: cannot convert value of type '(Int, Int, Int, Int, Int)' to expected argument type '(Int, Int, Int, Int)'
        return input.maxPooled(kernelSize: poolSize, strides: strides, padding: padding)