Skip to content

[AutoDiff] Added support for 'withoutDerivative(at:)'. #25691

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jun 23, 2019

Conversation

eaplatanios
Copy link

@rxwei following from our discussion in tensorflow/swift-apis#150.

@eaplatanios
Copy link
Author

One question that remains is, given these new helpers do we want to keep the Differentiable.withoutDerivative() helper? Removing it would likely result in a more consistent API with respect to how we denote code segments without derivatives.

@rxwei
Copy link
Contributor

rxwei commented Jun 23, 2019

One question that remains is, given these new helpers do we want to keep the Differentiable.withoutDerivative() helper? Removing it would likely result in a more consistent API with respect to how we denote code segments without derivatives.

No, please feel free to remove it.

@eaplatanios
Copy link
Author

Sounds good. I'll do that but I need to know how to replace this. Is there something along the lines of fixItInsertAround(sourceLoc, { x in "withoutDerivative(at: \(x))")?

@rxwei
Copy link
Contributor

rxwei commented Jun 23, 2019

There is a fixItInsert (before) and a fixItInsertAfter. A call to each will work.

@eaplatanios
Copy link
Author

Thanks! I made these edits and also changed the relevant tests. I cannot currently test locally so could you please run the CI tests so I check if the error reporting test modifications are correct?

@rxwei
Copy link
Contributor

rxwei commented Jun 23, 2019

@swift-ci please test tensorflow

1 similar comment
@rxwei
Copy link
Contributor

rxwei commented Jun 23, 2019

@swift-ci please test tensorflow

@eaplatanios
Copy link
Author

The CI errors are due to swift-apis. I'll make a corresponding PR there and try to test locally a bit later today.

@eaplatanios
Copy link
Author

@rxwei Tests seem to pass locally now. Could you please start the CI tests?

@rxwei
Copy link
Contributor

rxwei commented Jun 23, 2019

@swift-ci please test tensorflow

1 similar comment
@rxwei
Copy link
Contributor

rxwei commented Jun 23, 2019

@swift-ci please test tensorflow

@rxwei
Copy link
Contributor

rxwei commented Jun 23, 2019

@swift-ci please clean test tensorflow

@rxwei rxwei merged commit 70d7cc7 into swiftlang:tensorflow Jun 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants