Skip to content

Conversation

@rxwei
Copy link
Contributor

@rxwei rxwei commented Oct 1, 2019

Reads or writes to @noDerivative properties of struct should not be differentiated. At the implementation level, this amounts to not propagating activity through @noDerivative projections. This has been done in some of usefulness propagation, but there are two places where this has not been done:

  • In DifferentiableActivityInfo::propagateUsefulThroughBuffer, usefulness was being propagated arbitrarily regardless of the instruction.
  • In DifferentiableActivityInfo::recursivelySetVaried, variedness was set regardless of whether the proejction is a @noDerivative one.

This patch changes activity analysis to not propagate activity through @noDerivative field projections from a struct, covering both previously uncovered places discussed above. As a result, PullbackEmitter::getAdjointBuffer no longer needs to check for @noDerivative or emit any diagnostics.

Resolves TF-865.

Reads or writes to `@noDerivative` properties of struct should not be differentiated. At the implementation level, this amounts to not propagating activity through `@noDerivative` projections. This has been done in some of usefulness propagation, but there are two places where this has not been done:
* In `DifferentiableActivityInfo::propagateUsefulThroughBuffer`, usefulness was being propagated arbitrarily regardless of the instruction.
* In `DifferentiableActivityInfo::recursivelySetVaried`, variedness was set regardless of whether the proejction is a `@noDerivative` one.

This patch changes activity analysis to not propagate activity through `@noDerivative` field projections from a struct, covering both previously uncovered places discussed above. As a result, `PullbackEmitter::getAdjointBuffer` no longer needs to check for `@noDerivative` or emit any diagnostics.

Resolves [TF-865](https://bugs.swift.org/browse/TF-865).
@rxwei rxwei added the tensorflow This is for "tensorflow" branch PRs. label Oct 1, 2019
@rxwei rxwei requested a review from dan-zheng October 1, 2019 22:50
var tmp = s
// expected-note @+1 {{cannot differentiate through a '@noDerivative' stored property; do you want to use 'withoutDerivative(at:)'?}}
tmp.y = tmp.x
tmp.y = tmp.x // No diagnostics expected.
Copy link
Contributor

Choose a reason for hiding this comment

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

Big fix for semantics 👍

@rxwei
Copy link
Contributor Author

rxwei commented Oct 1, 2019

@swift-ci please test tensorflow

@rxwei rxwei merged commit fc1b278 into swiftlang:tensorflow Oct 1, 2019
@rxwei rxwei deleted the fix-noderivative-extract-activity branch October 2, 2019 00:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tensorflow This is for "tensorflow" branch PRs.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants