[AutoDiff] Forward-mode support for variables, generics, tuples, structs. #26743
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bartchr808
commented
Aug 22, 2019
bartchr808
force-pushed
the
differential-emitter-generic-support
branch
from
e58d9c6 to
b3a80a8
Compare
rxwei
reviewed
Aug 23, 2019
Contributor
Author
|
@swift-ci please test tensorflow |
1 similar comment
Contributor
Author
|
@swift-ci please test tensorflow |
Contributor
Author
|
@rxwei @dan-zheng just so it's easier for you to merge it in once we have time, spent a minute to deal with the merge conflict and verify it still works 😄 |
Contributor
Author
|
@swift-ci please test tensorflow |
Contributor
|
@swift-ci Please clean test tensorflow |
rxwei
reviewed
Sep 18, 2019
…-emitter-generic-support
Follow-up to swiftlang#26928 for reverse-mode. TF-788 tracks re-enabling the warning.
- Handle `copy_value` and `destroy_addr` in "should differentiate" logic. These should be visited if they have an active operand. - Add differential visitors for `destroy_value` and `destroy_addr`.
Remove precondition that argument must be tuple-typed to simplify call sites. Clean up call site code and doc comments.
dan-zheng
force-pushed
the
differential-emitter-generic-support
branch
from
36a1083 to
b86bfd4
Compare
dan-zheng
changed the title
TF-800 tracks special-case activity analysis and "should differentiate" logic regarding tuple-typed `apply` results.
dan-zheng
force-pushed
the
differential-emitter-generic-support
branch
from
b86bfd4 to
d3155ed
Compare
Contributor
Contributor
|
@swift-ci Please clean test tensorflow |
Contributor
|
Merging to unblock high priority patch #26709. |
rxwei
added a commit
to rxwei/swift
that referenced
this pull request
Sep 29, 2019
Differentiation conditions were inconsistent between JVP and VJP. Originally, `shouldDifferentiateInstruction` blindly differentiated any side-effecting instruction that has active operands. This worked well with VJP but not with JVP, so swiftlang#26743 added more specific conditions for JVP, but they did not work well with VJP because activity analysis was not yet unified. Now that activity analysis has been unified by swiftlang#27358, `shouldDifferentiateInstruction` no longer needs to distinguish between JVP and VJP, and the JVP differentiation conditions now apply to VJP as well. This PR makes both JVP and VJP use the same differentiation conditions. All existing tests pass. Fixes [TF-800](https://bugs.swift.org/browse/TF-800).
rxwei
added a commit
that referenced
this pull request
Sep 29, 2019
#27421) Differentiation conditions were inconsistent between JVP and VJP. Originally, `shouldDifferentiateInstruction` blindly differentiated any side-effecting instruction that has active operands. This worked well with VJP but not with JVP, so #26743 added more specific conditions for JVP, but they did not work well with VJP because activity analysis was not yet unified. Now that activity analysis has been unified by #27358, `shouldDifferentiateInstruction` no longer needs to distinguish between JVP and VJP, and the JVP differentiation conditions now apply to VJP as well. This PR makes both JVP and VJP use the same differentiation conditions. All existing tests pass. Fixes [TF-800](https://bugs.swift.org/browse/TF-800).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR will support everything other than control flow (coming in a PR later next week 🚀) and arrays for forward mode differentiation that was initially supported in #26057.
Commit history is completely messed up due to having to work off separate branches due to some progress blocking bug problems unfortunately 😄