Fast style transfer example #191
Conversation
|
Check out this pull request on ReviewNB: https://app.reviewnb.com/tensorflow/swift-models/pull/191 You'll be able to see notebook diffs and discuss changes. Powered by ReviewNB. |
There was a problem hiding this comment.
Thanks for the patch. I left some high level comments on coding style. Could you please make sure that the Swift API Design Guidelines and the Google Swift Style Guide are applied to everything in this PR?
|
Hi @rxwei, I've tried to spot & fix similar issues manually, but do you use some kind of 'official' linter to make this job automatic? |
|
@vvmnnnkv - For formatting, I use Might not work as well on notebooks, however. |
|
@BradLarson - interesting, https://github.com/apple/swift-format crashes when compiled in Ubuntu (with DEVELOPMENT-SNAPSHOT-2019-07-10-a). I'm not sure if this repo is the right |
|
@vvmnnnkv - Yes, I've seen similar problems with Swift for TensorFlow toolchains vs. stock Swift toolchains on swift-format. I think it's something that's being looked into. In the meantime, using swift-format with the stock toolchain should be fine. We do know that it has some problems around a few specific cases, so you may need to hand-correct in a couple of places. It works well overall, but isn't perfect. I know those cases are being worked on. |
# Conflicts: # .gitignore # Package.swift
|
@rxwei @BradLarson |
| @differentiable | ||
| public func callAsFunction(_ input: Tensor<Scalar>) -> Tensor<Scalar> { | ||
| // Calculate mean & variance along H,W axes. | ||
| let mean = input.mean(alongAxes: [1, 2]) |
There was a problem hiding this comment.
moments computes mean and variance simultaneously.
https://github.com/tensorflow/swift-apis/blob/c7595c4b4b0e824cd6d9449e2bed39944fafb472/Sources/TensorFlow/Operators/Math.swift#L2501-L2512
(variance computes mean internally so moments is some more efficient)
| /// 2-D layer applying instance normalization over a mini-batch of inputs. | ||
| /// | ||
| /// Reference: [Instance Normalization](https://arxiv.org/abs/1607.08022) | ||
| public struct InstanceNorm2D<Scalar: TensorFlowFloatingPoint>: Layer { |
There was a problem hiding this comment.
How about adding InstanceNorm2D to swift-apis?
Makes Tensor.padded() on par with tf.pad(). Added padding modes (reflect, symmetric) can be used to make "resize-convolution" layers like in tensorflow/swift-models#191.
Makes Tensor.padded() on par with tf.pad(). Added padding modes (reflect, symmetric) can be used to make "resize-convolution" layers like in tensorflow/swift-models#191.
|
Sorry it has been so long since we've reviewed this. I quick resolved the couple of merge conflicts, but it looks like a few areas of this model might need to be brought up to date to get this to build under the current toolchain. The warnings don't look too hard to fix (Raw has been renamed to _Raw, If you can get this to build cleanly under the current toolchain, I think this would be good to pull in. It looks like all previous comments have been addressed. |
|
Hi @BradLarson |
|
We're looking into the test failure, which looks to be a toolchain artifact. Once we get that resolved, I'll re-run CI and make sure everything's green. Sorry for the delay due to the U.S. holiday. |
|
We think we have a workaround for the failing test. Could you bring this branch up to date with the head of swift-models? I think that should allow for the tests to pass, with Marc's workaround that he committed yesterday. |
Example model for issue #146