Skip to content

[AutoDiff] Clean up @transposing attribute type-checking. #27988

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 1 commit into from
Oct 31, 2019

Conversation

dan-zheng
Copy link
Contributor

Remove dead code and unused parameters, unify style.

Fix capacity of @transposing attribute parameter indices.
The correct capacity is: non-wrt parameter count + wrt parameter count - 1.

Todos:

  • More clean up; this clean up was not comprehensive.
  • Consider revamping type-checking for instance methods so that
    @transposing functions can always be declared in same type
    context as their original declaration.

Remove dead code and unused parameters, unify style.

Fix capacity of `@transposing` attribute parameter indices.
The correct capacity is:
non-wrt parameter count + wrt parameter count - 1.

Todos:
- More clean up; this clean up was not comprehensive.
- Consider revamping type-checking for instance methods so that
  `@transposing` functions can always be declared in same type
  context as their original declaration.
@dan-zheng dan-zheng added the tensorflow This is for "tensorflow" branch PRs. label Oct 31, 2019
@dan-zheng dan-zheng requested review from rxwei and marcrasi October 31, 2019 04:56
@dan-zheng
Copy link
Contributor Author

@swift-ci Please test tensorflow

numUncurriedParams += resultFnType->getNumParams();
}
auto numParams = numUncurriedParams + parsedWrtParams.size() - 1;
auto parameterBits = SmallBitVector(numParams);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: @transposing attribute parameter indices capacity is fixed here.
Correct capacity: non-wrt parameter count + wrt parameter count - 1.

@@ -3954,9 +3904,14 @@ void AttributeChecker::visitTransposingAttr(TransposingAttr *attr) {
return;
}

bool wrtSelf = false;
if (!parsedWrtParams.empty())
wrtSelf = parsedWrtParams.front().getKind() ==
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Todo: make this wrtSelf logic more robust than checking parsedWrtParams.front().getKind().

@dan-zheng dan-zheng merged commit e4c8c8e into swiftlang:tensorflow Oct 31, 2019
@dan-zheng dan-zheng deleted the transposing-attr-cleanup branch October 31, 2019 18:32
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.

2 participants