Skip to content

[Serialization] Implement serialization for @differentiable attribute. #17155

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 6 commits into from
Jun 13, 2018

Conversation

dan-zheng
Copy link
Contributor

@dan-zheng dan-zheng commented Jun 12, 2018

Implement serialization/deserialization for all components of @differentiable attribute except the trailing where clause (which needs to be type-checked).

This is a necessary step for the #adjoint expression to look up @differentiable attributes declared on functions in other modules.

Addresses SR-7977.

@dan-zheng dan-zheng added the tensorflow This is for "tensorflow" branch PRs. label Jun 12, 2018
@dan-zheng dan-zheng self-assigned this Jun 12, 2018
@dan-zheng dan-zheng requested a review from rxwei June 12, 2018 22:08
@dan-zheng
Copy link
Contributor Author

@swift-ci Please test tensorflow

@dan-zheng dan-zheng force-pushed the serialize-autodiff-attr branch 2 times, most recently from db622e5 to 1c1e89c Compare June 12, 2018 22:30
Implement (de)serialization for all components of `@differentiable` attribute
except the trailing where clause (which needs to be type-checked).

This is a necessary step for the `#adjoint` expression to look up
`@differentiable` attributes declared on functions in other modules correctly.

Addresses SR-7977.
@dan-zheng dan-zheng force-pushed the serialize-autodiff-attr branch from 1c1e89c to d4da28d Compare June 12, 2018 22:30
DeclIDField, // Adjoint function declaration.
BCArray<BCFixed<32>> // Differentiation parameters.
>;

Copy link
Contributor

Choose a reason for hiding this comment

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

VERSION_MINOR in this file needs to be bumped.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! Done in d7ea8bc.

@dan-zheng dan-zheng force-pushed the serialize-autodiff-attr branch from 6cce8ef to d7ea8bc Compare June 12, 2018 22:38
Copy link
Contributor

@rxwei rxwei left a comment

Choose a reason for hiding this comment

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

Awesome work!

@@ -55,7 +55,7 @@ const uint16_t VERSION_MAJOR = 0;
/// describe what change you made. The content of this comment isn't important;
/// it just ensures a conflict if two people change the module format.
/// Don't worry about adhering to the 80-column limit for this line.
const uint16_t VERSION_MINOR = 405; // SWIFT_ENABLE_TENSORFLOW: graph_op.
const uint16_t VERSION_MINOR = 406 // SWIFT_ENABLE_TENSORFLOW: serialize @differentiable.
Copy link
Contributor

Choose a reason for hiding this comment

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

Semicolon

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, thanks.

for (auto paramValue : paramValues) {
auto parameter = paramValue & 0x01
? AutoDiffParameter::getSelfParameter(SourceLoc())
: AutoDiffParameter::getIndexParameter(SourceLoc(), paramValue>>1);
Copy link
Contributor

Choose a reason for hiding this comment

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

paramValue >> 1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, minor refactoring to keep things within 80 cols.
I too love spaces around operators.

Address comments from @rxwei.
@dan-zheng
Copy link
Contributor Author

@swift-ci Please test tensorflow

@dan-zheng
Copy link
Contributor Author

@swift-ci Please test tensorflow

@dan-zheng dan-zheng merged commit 32130d4 into swiftlang:tensorflow Jun 13, 2018
marcrasi pushed a commit that referenced this pull request Jun 14, 2018
#17155)

Implement (de)serialization for all components of `@differentiable` attribute
except the trailing where clause (which needs to be type-checked).

This is a necessary step for the `#adjoint` expression to look up
`@differentiable` attributes declared on functions in other modules correctly.

Addresses SR-7977.
marcrasi pushed a commit to google/swift that referenced this pull request Jun 15, 2018
swiftlang#17155)

Implement (de)serialization for all components of `@differentiable` attribute
except the trailing where clause (which needs to be type-checked).

This is a necessary step for the `#adjoint` expression to look up
`@differentiable` attributes declared on functions in other modules correctly.

Addresses SR-7977.
marcrasi pushed a commit that referenced this pull request Jun 22, 2018
#17155)

Implement (de)serialization for all components of `@differentiable` attribute
except the trailing where clause (which needs to be type-checked).

This is a necessary step for the `#adjoint` expression to look up
`@differentiable` attributes declared on functions in other modules correctly.

Addresses SR-7977.
marcrasi pushed a commit that referenced this pull request Jun 28, 2018
#17155)

Implement (de)serialization for all components of `@differentiable` attribute
except the trailing where clause (which needs to be type-checked).

This is a necessary step for the `#adjoint` expression to look up
`@differentiable` attributes declared on functions in other modules correctly.

Addresses SR-7977.
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