Skip to content

[AutoDiff] forbid derivative registration using @differentiable #30002

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 2 commits into from
Mar 24, 2020

Conversation

marcrasi
Copy link

@marcrasi marcrasi commented Feb 21, 2020

This is the tensorflow branch PR corresponding to #30001. Repeating the description:

Forbids the jvp: and vjp: arguments to @differentiable, deletes all the logic that handles them, and updates tests to use @derivative(of:) instead.

I left an error message saying that jvp: and vjp: are deprecated, so that there is a good explanation for anyone who missed the deprecation warning and whose code stops working. I think we should wait until next release to remove this.

I am running all the swift-apis, swift-models, swift-jupyter, swiftai, etc tests on this to catch any leftover jvp: or vjp: that we forgot to remove.

@marcrasi marcrasi requested review from rxwei and dan-zheng February 21, 2020 21:28
@compnerd compnerd closed this Feb 22, 2020
@dan-zheng
Copy link
Contributor

I accidentally deleted tensorflow branch, which closed this PR. That was not intentional, sorry!
It would be nice to protect tensorflow branch against deletion to prevent this from happening again.

@dan-zheng dan-zheng reopened this Feb 22, 2020
@marcrasi
Copy link
Author

Test run caught some 'vjp:' in OpenSpiel! I'll fix that before proceeding with merging this PR.

OpenSpiel pushed a commit to google-deepmind/open_spiel that referenced this pull request Feb 26, 2020
This change unblocks deprecation of @differentiable(vjp:) (swiftlang/swift#30002).

PiperOrigin-RevId: 297439933
Change-Id: I0aad361fc9ae7091d7e49670a95f91b103d0f117
@marcrasi marcrasi force-pushed the tf-remove-differentiable-jvp-vjp branch 2 times, most recently from ca3311b to ab83c95 Compare March 3, 2020 02:30
@marcrasi
Copy link
Author

marcrasi commented Mar 3, 2020

@swift-ci please test tensorflow

2 similar comments
@marcrasi
Copy link
Author

marcrasi commented Mar 4, 2020

@swift-ci please test tensorflow

@marcrasi
Copy link
Author

marcrasi commented Mar 4, 2020

@swift-ci please test tensorflow

@marcrasi
Copy link
Author

marcrasi commented Mar 4, 2020

This seems good to go now, but I'll wait until we have caught up with the master->tensorflow merge so that I don't further pile on conflicts.

@marcrasi marcrasi closed this Mar 23, 2020
@marcrasi marcrasi force-pushed the tf-remove-differentiable-jvp-vjp branch from a439d6f to 4544f5e Compare March 23, 2020 19:48
@marcrasi marcrasi reopened this Mar 23, 2020
@marcrasi marcrasi force-pushed the tf-remove-differentiable-jvp-vjp branch from 61a66ec to 88dabef Compare March 23, 2020 20:38
@marcrasi marcrasi force-pushed the tf-remove-differentiable-jvp-vjp branch from 88dabef to 7f2495e Compare March 23, 2020 20:45
@marcrasi
Copy link
Author

@swift-ci please test tensorflow

@dan-zheng
Copy link
Contributor

Merging now that upstream mirror #30001 has been merged.

@dan-zheng dan-zheng merged commit 0a7a548 into tensorflow Mar 24, 2020
@dan-zheng dan-zheng deleted the tf-remove-differentiable-jvp-vjp branch March 24, 2020 07:42
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.

4 participants