Skip to content
This repository was archived by the owner on Jul 1, 2023. It is now read-only.

[Linear Algebra] Trace operator #586

Merged
merged 14 commits into from
Dec 20, 2019
Merged

[Linear Algebra] Trace operator #586

merged 14 commits into from
Dec 20, 2019

Conversation

awav
Copy link
Contributor

@awav awav commented Dec 19, 2019 •
edited
Loading

The PR adds trace linear operator #562.

saeta
saeta approved these changes Dec 19, 2019
View reviewed changes
Copy link
Contributor

@saeta saeta left a comment

Choose a reason for hiding this comment

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

This looks really great! Thank you @awav .

@eaplatanios
Copy link
Contributor

Thanks @awav! You could also add the preconditions in the doc string using - Precondition:. We have not been consistent in doing that so far, but I think it helps to have them there.

@dan-zheng
Minor test cleanup.
4a508da 
- Remove `scalar` typealias for consistency with other tests.
- Avoid explicit `TensorShape` initialization.
- Avoid explicit `Tensor` generic parameter.
- Use idiomatic collection indexing APIs.
@dan-zheng
Minor test fix.
a46672a
dan-zheng
dan-zheng reviewed Dec 19, 2019
View reviewed changes
dan-zheng and others added 7 commits December 19, 2019 11:50
@dan-zheng
Update doc comments for trace.
6dfdc88 
Add precondition comment.
Use consistent wording with doc comment for `cholesky`.
Todo: further improve doc comment consistency.
@awav
Update documentation
0e22a87
@awav
Address comments about docstring and change squared shapes to bizzare…
93b842d 
… shapes
@awav
Update docstring
53a08d5
@awav
Add test for non-squared matrix
3133f9d
@awav
Correct shape for trace in docstring
2ee7217
@dan-zheng
Update doc comments for trace.
c1407b6 
Removed main diagonal definition clarification, since main diagonal is
well-defined for non-square matrices.
dan-zheng
dan-zheng approved these changes Dec 19, 2019
View reviewed changes
Copy link
Member

@dan-zheng dan-zheng left a comment

Choose a reason for hiding this comment

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

Thanks for adding a non-square matrix test!

@saeta saeta merged commit eebb487 into tensorflow:master Dec 20, 2019
@dan-zheng dan-zheng mentioned this pull request Dec 20, 2019
Open
15 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@saeta saeta saeta approved these changes

@dan-zheng dan-zheng dan-zheng approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@awav @eaplatanios @saeta @dan-zheng @kokoro-team