Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add
KernelTensorSum
#507base: master
Are you sure you want to change the base?
Add
KernelTensorSum
#507Changes from 3 commits
5a6037d
2be0d37
a39f81f
03ce5b5
106ef5f
668fcfd
d1a18b2
36b4d9e
8d0da0a
e016424
598fbc7
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems too generic to be defined and exported from KernelFunctions. Is it not part of TensorCore or some other lightweight interface package? We would also a non-Unicode alias, as for other keyword arguments and functions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about that, but
⊕
is not part of TensorCore or as far as I know any other lightweight package (https://juliahub.com/ui/Search?q=%E2%8A%95&type=symbols). It is a help constructor for the newKernelTensorSum
/KernelIndependentSum
, so the non-Unicode function is already available.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestions wellcome on how to improve this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no non-Unicode alternative similar to
+
,*
, ortensor
yet as far as I can tell?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah! I see, you're right
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resolve?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This problem is not fixed yet, is it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But since
TensorCore.jl
does not define⊕
, what should we do? Here are the packages that use⊕
.Kronecker.jl
is one, but I guess we do not want to add this as a dependency, only to re-use the symbol.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should make a PR to TensorCore. I think the operator should not be owned by KernelFunctions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
JuliaMath/TensorCore.jl#10