Skip to content

Conversation

@jekbradbury
Copy link

If a stored property of a struct with synthesized Differentiable
conformance is of a type that conforms to Differentiable, but is
marked @noDerivative, a struct_extract instruction that extracts
that property shouldn't propagate variedness to its result.

If a stored property of a struct with synthesized Differentiable 
conformance is of a type that conforms to Differentiable, but is marked 
`@noDerivative`, a struct_extract instruction that extracts that 
property shouldn't propagate variedness to its result.
@jekbradbury jekbradbury requested a review from rxwei January 16, 2019 00:53
@jekbradbury jekbradbury added the tensorflow This is for "tensorflow" branch PRs. label Jan 16, 2019
if (isVaried(cai->getSrc(), i))
recursivelySetVariedIfDifferentiable(cai->getDest(), i);
}
else if (auto *sei = dyn_cast<StructExtractInst>(&inst)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

You'll also want to check whether CotangentVector is @_fieldwiseProductSpace, because if it's not, we consider @noDerivative as having had no effect on conformance synthesis, so it could still have a VJP we would call when it's active.

It would be nice to move the code that checks for @_fieldwiseProductSpace into a helper.

Co-Authored-By: jekbradbury <jekbradbury@gmail.com>
@jekbradbury
Copy link
Author

@swift-ci test tensorflow

@jekbradbury
Copy link
Author

@swift-ci please test tensorflow

1 similar comment
@pschuh
Copy link
Contributor

pschuh commented Jan 16, 2019

@swift-ci please test tensorflow

@jekbradbury
Copy link
Author

Failure was a timeout. OK to merge @rxwei?

@rxwei
Copy link
Contributor

rxwei commented Jan 16, 2019

yeah

@jekbradbury jekbradbury merged commit 496ee3f into swiftlang:tensorflow Jan 16, 2019
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.

3 participants