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

Conversation

@dave-fernandes
Copy link

Added Conv1D, MaxPool1D and AvgPool1D layers

@dave-fernandes
Copy link
Author

The following PR must be merged before this one will compile:
swiftlang/swift#23355

/// - Parameters:
/// - filter: The filter (width, inputChannels, outputChannels).
/// - bias: The bias (dimensions: output channels).
/// - activation: The activation activation.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// - activation: The activation activation.
/// - activation: The element-wise activation function.

Please also make this change elsewhere, including for Conv2D.

/// padding.
///
/// - Parameters:
/// - filter: The filter (width, inputChannels, outputChannels).
Copy link
Member

Choose a reason for hiding this comment

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

For the - Parameters: doc comment of memberwise initializers, please copy the doc comments from each individual stored property.

For example:

/// - Parameters:
///   - filter: The 3-D convolution kernel.
///   - bias: The bias vector.
///   ...

Please also make this change elsewhere, including for Conv2D.

/// initialization with the specified seed. The bias vector is initialized with zeros.
///
/// - Parameters:
/// - filterShape: The shape of the filter (width, inputChannels, outputChannels).
Copy link
Member

Choose a reason for hiding this comment

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

Please use `[dim0, dim1, ...]` syntax for specifying all shapes in doc comments.

Suggested change
/// - filterShape: The shape of the filter (width, inputChannels, outputChannels).
/// - filterShape: The 3-D shape of the filter, representing
/// `[width, inputChannels, outputChannels]`.

public init(
filter: Tensor<Scalar>,
bias: Tensor<Scalar>,
activation: @escaping Activation,
Copy link
Contributor

Choose a reason for hiding this comment

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

We are using identity as the default activation in Dense and Conv2D.

Suggested change
activation: @escaping Activation,
activation: @escaping Activation = identity,

Copy link
Author

Choose a reason for hiding this comment

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

The init method in the struct declaration doesn't have a default. Only the ones in the extensions have default values. So this is consistent, but maybe not what is desired?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, you are right!

@differentiable
public func applied(to input: Tensor<Scalar>, in _: Context) -> Tensor<Scalar> {
return input.expandingShape(at: 1).maxPooled(
kernelSize: (1, 1, poolSize, 1), strides: (1, 1, stride, 1), padding: padding
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
kernelSize: (1, 1, poolSize, 1), strides: (1, 1, stride, 1), padding: padding
kernelSize: (1, 1, poolSize, 1), strides: (1, 1, stride, 1), padding: padding

@differentiable
public func applied(to input: Tensor<Scalar>, in _: Context) -> Tensor<Scalar> {
return input.expandingShape(at: 1).averagePooled(
kernelSize: (1, 1, poolSize, 1), strides: (1, 1, stride, 1), padding: padding
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
kernelSize: (1, 1, poolSize, 1), strides: (1, 1, stride, 1), padding: padding
kernelSize: (1, 1, poolSize, 1), strides: (1, 1, stride, 1), padding: padding

func testConv1D() {
let filter = Tensor<Float>(ones: [3, 1, 2]) * Tensor<Float>([[[0.33333333, 1]]])
let bias = Tensor<Float>([0, 1])
let layer = Conv1D<Float>(filter: filter, bias: bias, activation: identity, stride: 1, padding: .valid)
Copy link
Contributor

Choose a reason for hiding this comment

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

Fit within 100 columns.

let filterTensorShape = TensorShape([
Int32(filterShape.0), Int32(filterShape.1), Int32(filterShape.2)])
self.init(
filter: Tensor(glorotUniform: filterTensorShape, seed: seed),
Copy link
Contributor

Choose a reason for hiding this comment

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

Indent by 4 from self.

@dave-fernandes
Copy link
Author

Regarding the APIs, I think MaxPool2D and AvgPool2D should use 2-tuples instead of 4-tuples for strides and poolSize. The 4-tuples are inconsistent with Conv2D.

Copy link
Contributor

@rxwei rxwei 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 Dave!

@rxwei rxwei merged commit d87fab3 into tensorflow:master Mar 23, 2019
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.

3 participants