Skip to content
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

[Swift] Fix latent inout argument differentiation bugs. #113

Merged
merged 1 commit into from
Nov 20, 2019

Conversation

dan-zheng
Copy link
Member

@dan-zheng dan-zheng commented Nov 20, 2019

mutating functions like Array.append and ActionValueCalculator.loss
cannot be differentiated because differentiation does not yet support
function applications with inout arguments.

These latent bugs were uncovered in swiftlang/swift#28352,
which correctly diagnoses function applications with inout arguments.

https://bugs.swift.org/browse/TF-129 tracks inout argument support.
In the meantime, disable affected code and tests.


Manually tested with Swift for TensorFlow nightly development toolchains.
Unblocks Swift for TensorFlow CI.

`mutating` functions like `Array.append` and `ActionValueCalculator.loss`
cannot be differentiated because differentiation does not yet support
function applications with `inout` arguments.

These latent bugs were uncovered in swiftlang/swift#28352,
which correctly diagnoses function applications with `inout` arguments.

https://bugs.swift.org/browse/TF-129 tracks `inout` argument support.
In the meantime, disable affected code and tests.
@marcrasi
Copy link

Why does AD diagnose on these? It looks to me like the inout arguments aren't active so differentiation should work even without inout argument support.

OpenSpiel pushed a commit that referenced this pull request Nov 20, 2019
PiperOrigin-RevId: 281540491
Change-Id: Ib3bf1ae3fece0f664dc13da23df69a077c883665
@OpenSpiel OpenSpiel merged commit 594c9be into google-deepmind:master Nov 20, 2019
@dan-zheng
Copy link
Member Author

Why does AD diagnose on these? It looks to me like the inout arguments aren't active so differentiation should work even without inout argument support.

Thanks for the catch! It's likely this patch wasn't correct, I'll investigate further.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants