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

remove do_queue argument from Operation.adjoint #2583

Merged
merged 6 commits into from
May 18, 2022
Merged

Conversation

albi3ro
Copy link
Contributor

@albi3ro albi3ro commented May 16, 2022

The Operation.adjoint method currently has a do_queue keyword that is not used for anything and is usually left out of child implementations.

This PR standardizes the removal of the do_queue keyword.

@albi3ro albi3ro requested a review from Jaybsoni May 16, 2022 21:02
@codecov
Copy link

codecov bot commented May 16, 2022

Codecov Report

Merging #2583 (3d1f229) into master (6892a0c) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #2583   +/-   ##
=======================================
  Coverage   99.58%   99.58%           
=======================================
  Files         243      243           
  Lines       19595    19595           
=======================================
  Hits        19514    19514           
  Misses         81       81           
Impacted Files Coverage Δ
pennylane/operation.py 96.73% <100.00%> (ø)
pennylane/ops/cv.py 100.00% <100.00%> (ø)
pennylane/ops/identity.py 100.00% <100.00%> (ø)
pennylane/ops/qubit/non_parametric_ops.py 100.00% <100.00%> (ø)
pennylane/ops/snapshot.py 100.00% <100.00%> (ø)
pennylane/templates/embeddings/amplitude.py 98.00% <100.00%> (ø)
pennylane/templates/layers/cv_neural_net.py 100.00% <100.00%> (ø)
...ylane/templates/subroutines/commuting_evolution.py 100.00% <100.00%> (ø)
pennylane/templates/subroutines/hilbert_schmidt.py 100.00% <100.00%> (ø)
pennylane/templates/subroutines/interferometer.py 100.00% <100.00%> (ø)
... and 1 more

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 bcd837b...3d1f229. Read the comment docs.

Copy link
Contributor

@Jaybsoni Jaybsoni left a comment

Choose a reason for hiding this comment

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

Looks pretty straight forward! Would this be removing the ability to control if an adjoint operation is queued completely, or would we just move that functionality to the Adjoint wrapper class?

It might be good to check (I'm not sure) if there are any tests that need to be updated or removed that check for queuing of the adjoint of these operations. But other then that I think its good to go

@albi3ro
Copy link
Contributor Author

albi3ro commented May 18, 2022

Obviously, this keyword was completely untested, as I could remove it without breaking any tests. Also, the most common operations already had it removed, leading to lots of manually silenced linting complaints.

@albi3ro albi3ro merged commit 48636c5 into master May 18, 2022
@albi3ro albi3ro deleted the adjoint_do_queue branch May 18, 2022 15:24
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.

None yet

2 participants