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

Move is_commuting to pennylane/ops/functions #2991

Merged
merged 7 commits into from
Aug 23, 2022

Conversation

albi3ro
Copy link
Contributor

@albi3ro albi3ro commented Aug 23, 2022

I need to figure out how is_commuting works to extend it for arithmetic ops like Controlled.

While it was created in the development of the CommutationDAG and used by the commutation dag, pennylane/transforms/commutation_dag.py is a fairly non-intuitive place for a top-level function is_commuting.

As it is a function of operators, this PR moves it to where we place our functions of operators.

No changes are made to the source code, though we should try and improve how this function works.

@albi3ro albi3ro added the review-ready 👌 PRs which are ready for review by someone from the core team. label Aug 23, 2022
@codecov
Copy link

codecov bot commented Aug 23, 2022

Codecov Report

Merging #2991 (d00f7bf) into master (352947b) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #2991   +/-   ##
=======================================
  Coverage   99.65%   99.65%           
=======================================
  Files         266      267    +1     
  Lines       22397    22400    +3     
=======================================
+ Hits        22320    22323    +3     
  Misses         77       77           
Impacted Files Coverage Δ
pennylane/__init__.py 100.00% <ø> (ø)
pennylane/transforms/__init__.py 100.00% <ø> (ø)
pennylane/ops/functions/__init__.py 100.00% <100.00%> (ø)
pennylane/ops/functions/is_commuting.py 100.00% <100.00%> (ø)
pennylane/transforms/commutation_dag.py 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

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 good to me !
Just one comment, in the documentation we have a subheading Quantum Operators where we list operator functions like is_hermitian and is_unitary. It might be worth adding this to that table

@albi3ro albi3ro requested a review from Jaybsoni August 23, 2022 14:35
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.

Ready to go 👍🏼

@albi3ro albi3ro merged commit 1e3a731 into master Aug 23, 2022
@albi3ro albi3ro deleted the move-is_commuting-ops-functions branch August 23, 2022 20:43
@rmoyard rmoyard added this to the v0.26.0 milestone Sep 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
review-ready 👌 PRs which are ready for review by someone from the core team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants