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

Allow broadcasting in the numerical representations of standard operations #2609

Merged
merged 26 commits into from
Jun 2, 2022

Conversation

dwierichs
Copy link
Contributor

This PR is a duplicate of #2535 that follows the conventions set up in #2575 and #2590 instead.
It sets the ndim_params attribute for many standard operations and allows for broadcasting in their numerical representations (mostly compute_matrix and compute_eigvals).

@dwierichs
Copy link
Contributor Author

[sc-18891]

@codecov
Copy link

codecov bot commented May 24, 2022

Codecov Report

Merging #2609 (0a454f0) into parameter-broadcasting-1 (ab69c3e) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@                    Coverage Diff                     @@
##           parameter-broadcasting-1    #2609    +/-   ##
==========================================================
  Coverage                     99.58%   99.58%            
==========================================================
  Files                           245      245            
  Lines                         19700    19823   +123     
==========================================================
+ Hits                          19618    19741   +123     
  Misses                           82       82            
Impacted Files Coverage Δ
pennylane/math/single_dispatch.py 98.85% <100.00%> (+<0.01%) ⬆️
pennylane/operation.py 96.58% <100.00%> (+0.04%) ⬆️
pennylane/ops/functions/matrix.py 100.00% <100.00%> (ø)
pennylane/ops/qubit/attributes.py 100.00% <100.00%> (ø)
pennylane/ops/qubit/matrix_ops.py 100.00% <100.00%> (ø)
pennylane/ops/qubit/parametric_ops.py 100.00% <100.00%> (ø)

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 ab69c3e...0a454f0. Read the comment docs.

@dwierichs dwierichs requested a review from josh146 May 27, 2022 06:15
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.

An amazing PR @dwierichs 💪

I left mostly questions for my own understanding for the main logic additions.

Regarding the tests, it would be great if existing tests could be left as-is where it makes sense, and new test classes TestSomethingBatched: were added? This would provide some peace of mind that existing functionality is preserved, while also adding tests that are explicitly testing the batched functionality.

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/math/single_dispatch.py Show resolved Hide resolved
pennylane/operation.py Show resolved Hide resolved
pennylane/ops/qubit/parametric_ops.py Outdated Show resolved Hide resolved
tests/devices/test_default_qubit.py Show resolved Hide resolved
tests/ops/qubit/test_attributes.py Outdated Show resolved Hide resolved
tests/ops/qubit/test_matrix_ops.py Outdated Show resolved Hide resolved
tests/ops/qubit/test_parametric_ops.py Outdated Show resolved Hide resolved
dwierichs and others added 2 commits May 30, 2022 15:56
Co-authored-by: Josh Izaac <josh146@gmail.com>
@dwierichs
Copy link
Contributor Author

dwierichs commented May 31, 2022

Thanks for the review, @josh146 !
I took everything into account and split up the tests into ..._broadcasted and original versions. (I also tried to avoid the word "batch" for broadcasting, to reduce confusions and separate the new parameter broadcasting from the more general concept of a batch as in batch_transform or batch_execute)
New issues resulted from this PR:

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.

Amazing work @dwierichs! Thanks for revising the tests, they now look 👌

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
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
doc/releases/changelog-dev.md Show resolved Hide resolved
pennylane/operation.py Show resolved Hide resolved
pennylane/ops/qubit/matrix_ops.py Show resolved Hide resolved
pennylane/ops/qubit/parametric_ops.py Show resolved Hide resolved
dwierichs and others added 2 commits June 2, 2022 16:28
Co-authored-by: Josh Izaac <josh146@gmail.com>
@dwierichs dwierichs merged commit fe73c3c into parameter-broadcasting-1 Jun 2, 2022
@dwierichs dwierichs deleted the parameter-broadcasting-2 branch June 2, 2022 15:25
dwierichs added a commit that referenced this pull request Jun 3, 2022
…oadcasting (#2590)

* introduce Operator.ndim_params, Operator.batch_size, QuantumTape.batch_size

* linting

* changelog

* enable tf.function input_signature usage

* black

* test for unsilenced error

* Apply suggestions from code review

Co-authored-by: Josh Izaac <josh146@gmail.com>

* introduce device flag and batch_transform for unbroadcasting; use transform in device.batch_transform

* black, [skip ci]

* code review

* string formatting [skip ci]

* operation broadcasting interface tests

* unbroadcast_expand

* tests for expand function

* tests

* black

* compatibility with TensorFlow 2.6

* builtins unstack

* failing case coverage

* stop using I in operation.py [skip ci]

* Apply suggestions from code review

Co-authored-by: Josh Izaac <josh146@gmail.com>

* review

* Apply suggestions from code review

Co-authored-by: Josh Izaac <josh146@gmail.com>

* review [skip ci]

* move changelog section from "improvements" to "new features"

* changelog

* add missing files

* namespace

* linting variable names

* pin protobuf<4.21.0

* docstring

* unpin protobuf

* Allow broadcasting in the numerical representations of standard operations (#2609)

* commit old changes

* intermed

* clean up, move broadcast dimension first

* update tests that manually set ndim_params for default ops

* pin protobuf<4.21.0

* improve shape coersion order

* changelog formatting

* broadcasted pow tests

* attribute test, ControlledQubitUnitary update

* test kwargs attributes

* Apply suggestions from code review

Co-authored-by: Josh Izaac <josh146@gmail.com>

* changelog

* review

* remove prints

* explicit attribute supports_broadcasting tests

* tests disentangle

* fix

* PauliRot broadcasted identity compatible with TF

* rename "batched" into "broadcasted" for uniform namespace

* old TF version support in qubitunitary unitarity check

* python3.7 support

* Apply suggestions from code review

Co-authored-by: Josh Izaac <josh146@gmail.com>

* linebreak

Co-authored-by: Josh Izaac <josh146@gmail.com>

* black

* black again

* feature collision amend tests

* black [skip ci]

Co-authored-by: Josh Izaac <josh146@gmail.com>
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.

3 participants