Skip to content
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

Introduce operator input parameter ndim_params and batch_size attributes #2575

Merged
merged 18 commits into from
May 24, 2022

Conversation

dwierichs
Copy link
Contributor

This PR implements the first part of parameter broadcasting, including the operator attributes ndim_params and batch_size (which is based on _batch_size) as well as the tape attribute batch_size which is inferred from the batch sizes of the tape operations.
These attributes will be used to determine if an operator or a tape is batched, which will be used by devices at execution.

@codecov
Copy link

codecov bot commented May 15, 2022

Codecov Report

Merging #2575 (2e68fc9) into master (a61e25a) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #2575   +/-   ##
=======================================
  Coverage   99.58%   99.58%           
=======================================
  Files         243      243           
  Lines       19589    19641   +52     
=======================================
+ Hits        19508    19560   +52     
  Misses         81       81           
Impacted Files Coverage Δ
pennylane/math/single_dispatch.py 98.84% <100.00%> (+0.02%) ⬆️
pennylane/operation.py 96.84% <100.00%> (+0.11%) ⬆️
pennylane/ops/channel.py 100.00% <100.00%> (ø)
pennylane/tape/tape.py 98.89% <100.00%> (+0.05%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a61e25a...2e68fc9. Read the comment docs.

@dwierichs
Copy link
Contributor Author

[sc-19788]

@dwierichs dwierichs marked this pull request as ready for review May 17, 2022 08:00
@dwierichs dwierichs requested a review from josh146 May 17, 2022 08:00
@dwierichs dwierichs changed the title Introduce operator input parameter ndim_params and batch_size attributes [broadcasting] Introduce operator input parameter ndim_params and batch_size attributes May 17, 2022
@dwierichs dwierichs changed the title [broadcasting] Introduce operator input parameter ndim_params and batch_size attributes Introduce operator input parameter ndim_params and batch_size attributes May 17, 2022
Copy link
Member

@josh146 josh146 left a comment

Choose a reason for hiding this comment

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

Thanks @dwierichs! Wow, this is a really clean PR, very nice! I left some comments and questions, but all minor - I think the main components of the PR are in very good shape

pennylane/math/single_dispatch.py Show resolved Hide resolved
pennylane/operation.py Show resolved Hide resolved
pennylane/operation.py Outdated Show resolved Hide resolved
pennylane/operation.py Outdated Show resolved Hide resolved
pennylane/operation.py Outdated Show resolved Hide resolved
pennylane/tape/tape.py Outdated Show resolved Hide resolved
pennylane/tape/tape.py Show resolved Hide resolved
tests/tape/test_tape.py Show resolved Hide resolved
pennylane/operation.py Show resolved Hide resolved
tests/test_operation.py Outdated Show resolved Hide resolved
Co-authored-by: Josh Izaac <josh146@gmail.com>
Copy link
Contributor Author

@dwierichs dwierichs 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 the quick review! :)

pennylane/math/single_dispatch.py Show resolved Hide resolved
pennylane/operation.py Show resolved Hide resolved
pennylane/operation.py Outdated Show resolved Hide resolved
pennylane/operation.py Show resolved Hide resolved
pennylane/operation.py Outdated Show resolved Hide resolved
pennylane/operation.py Outdated Show resolved Hide resolved
pennylane/tape/tape.py Show resolved Hide resolved
tests/test_operation.py Outdated Show resolved Hide resolved
Copy link
Member

@josh146 josh146 left a comment

Choose a reason for hiding this comment

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

Nice work @dwierichs! Happy for this to be merged when my suggestions are resolved :)

doc/releases/changelog-dev.md Outdated Show resolved Hide resolved
doc/releases/changelog-dev.md Outdated Show resolved Hide resolved
doc/releases/changelog-dev.md Outdated Show resolved Hide resolved
pennylane/operation.py Show resolved Hide resolved
pennylane/operation.py Show resolved Hide resolved
pennylane/ops/channel.py Show resolved Hide resolved
pennylane/tape/tape.py Show resolved Hide resolved
pennylane/tape/tape.py Show resolved Hide resolved
pennylane/tape/tape.py Outdated Show resolved Hide resolved
dwierichs and others added 2 commits May 24, 2022 10:06
Co-authored-by: Josh Izaac <josh146@gmail.com>
@dwierichs
Copy link
Contributor Author

@josh146 Thanks for the second review! There are 2 open conversations that should not require any additional action, I think.
Let me know whether they are fine and I'll resolve and merge :)

@dwierichs
Copy link
Contributor Author

[sc-19788]

@josh146
Copy link
Member

josh146 commented May 24, 2022

Resolved :)

@dwierichs dwierichs merged commit 8593ad7 into master May 24, 2022
@dwierichs dwierichs deleted the parameter-broadcasting-0 branch May 24, 2022 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants