-
Notifications
You must be signed in to change notification settings - Fork 575
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
Use the new multi_dispatch
decorator in the math module. Add tensordot
tests.
#2096
Merged
Merged
Changes from 5 commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
a5e2746
use multi_dispatch decorator across math module
dwierichs 308ddef
tests
dwierichs 270837d
more tests
dwierichs fe6b027
changelog
dwierichs 8ac4d09
Merge branch 'master' into multi_dispatch-usage
dwierichs ea4296b
filename type
dwierichs f02fc56
filename typo
dwierichs a8e1604
Merge branch 'multi_dispatch-usage' of github.com:PennyLaneAI/pennyla…
dwierichs cd8ac5a
intermediate
dwierichs b80bb81
Merge branch 'master' into multi_dispatch-usage
dwierichs 5b3b255
Merge branch 'multi_dispatch-usage' of github.com:PennyLaneAI/pennyla…
dwierichs 2c7ef26
tmp check
dwierichs 12abf98
reduce warnings across math module test suite
dwierichs 3afc884
include multi_dispatch applications, again.
dwierichs 58c6661
Merge branch 'master' into multi_dispatch-usage
dwierichs File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
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.
@dwierichs is it possible to modify the decorator to allow for 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.
I'm not sure. It would have to
try
to iterate over the arguments in (via the list methodextend
) that are marked viatensor_list
, and if it fails, attempt to justappend
the respective argument to the arguments to dispatch over.I'd consider this an unreasonable overhead given the currently single use case. What do you think?
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.
Oh, I was thinking something simpler, although maybe it is not in the spirit of 'multiple dispatch'.
Basically, check
not isinstance(arg, (list, tuple))
, and if this is the case, callqml.math.get_interface(arg)
.Basically, we assume that a
tensor_list
argument must be a built-in Python sequence (either list or tuple), and if this is not the case, we assume it must be a tensor and simply single dispatchThere 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, that sounds cool! I'll give it a go :)
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 implemented this now, a bit more lightweight, even: If an argument marked as tensor list is not a
tuple
orlist
, it is simply treated as if it was not marked as a tensor list.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, nice call!