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

✨ Add XX-plus-YY and XX-minus-YY gate support to ZX library #482

Merged
merged 7 commits into from
Feb 12, 2024

Conversation

burgholzer
Copy link
Member

@burgholzer burgholzer commented Nov 14, 2023

Description

This PR adds the last remaining unsupported gates to the ZX library.
In the process, it also introduces a new convenience function for integer division of parameter expressions that simplifies some code. In conjunction with #549, this also fixes #486.

Fixes #343

Checklist:

  • The pull request only contains commits that are related to it.
  • I have added appropriate tests and documentation.
  • I have made sure that all CI jobs on GitHub pass.
  • The pull request introduces no new warnings and follows the project's style guidelines.

@burgholzer burgholzer added enhancement New feature or request ZX Anything related to the ZX package c++ Anything related to C++ code labels Nov 14, 2023
@burgholzer burgholzer added this to the ZX Package Improvements milestone Nov 14, 2023
@burgholzer burgholzer self-assigned this Nov 14, 2023
@burgholzer burgholzer linked an issue Nov 14, 2023 that may be closed by this pull request
Copy link

codecov bot commented Nov 14, 2023

Codecov Report

Attention: 13 lines in your changes are missing coverage. Please review.

Comparison is base (af9d20a) 91.1% compared to head (24670ee) 91.1%.

Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##            main    #482   +/-   ##
=====================================
  Coverage   91.1%   91.1%           
=====================================
  Files        131     131           
  Lines      13667   13761   +94     
  Branches    2150    2166   +16     
=====================================
+ Hits       12451   12539   +88     
- Misses      1216    1222    +6     
Flag Coverage Δ
cpp 90.8% <90.9%> (+<0.1%) ⬆️
python 99.5% <ø> (ø)
Files Coverage Δ
include/mqt-core/operations/Expression.hpp 92.7% <90.0%> (-0.2%) ⬇️
src/zx/FunctionalityConstruction.cpp 93.3% <90.9%> (-1.6%) ⬇️

... and 2 files with indirect coverage changes

Copy link
Member

@pehamTom pehamTom left a comment

Choose a reason for hiding this comment

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

LGTM. I checked with the qiskit decompositions and everything seems alright with me. Just a tiny optimization but for me its fine to omit it.

Another thing that I thought of is that Z rotations can be fused with the Control of CNOTs before or after (same for X rotations and targets). But I think that happens in more places than the added gates so it can be handled separately.

src/zx/FunctionalityConstruction.cpp Outdated Show resolved Hide resolved
src/zx/FunctionalityConstruction.cpp Outdated Show resolved Hide resolved
@burgholzer
Copy link
Member Author

LGTM. I checked with the qiskit decompositions and everything seems alright with me. Just a tiny optimization but for me its fine to omit it.

Another thing that I thought of is that Z rotations can be fused with the Control of CNOTs before or after (same for X rotations and targets). But I think that happens in more places than the added gates so it can be handled separately.

Thanks for the feedback! I remember that I even tried that from the very beginning but I was getting some kind of weird results and couldn't get tests to pass.
Now that investigated a little more, I stumbled upon #486, which is kind of unfortunate and explains the errors I got on my first try.

burgholzer and others added 6 commits February 12, 2024 16:17
Signed-off-by: burgholzer <burgholzer@me.com>
Signed-off-by: burgholzer <burgholzer@me.com>
Signed-off-by: burgholzer <burgholzer@me.com>
Signed-off-by: burgholzer <burgholzer@me.com>
Signed-off-by: burgholzer <burgholzer@me.com>
When floating point values are converted to `PiRational`s, they are constrained to the interval (-pi, pi]. This doesn't
matter during computations in the ZX-library, but when converting from a quantum circuit, this can lead to problems when
dealing with global phases. If global phases have to be introduced while parsing a `QuantumComputation, ' the original value of the phase must be passed along so the global phase is set correctly.
Integer Division for Expressions was accidentally removed during rebase.
Copy link
Member Author

@burgholzer burgholzer left a comment

Choose a reason for hiding this comment

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

@pehamTom many thanks for the changes here. Nice to see this go in!
Looking forward to the next QCEC release with the additional gate support and these fixes.

@burgholzer burgholzer merged commit 5f2041d into main Feb 12, 2024
35 checks passed
@burgholzer burgholzer deleted the xx-pm-yy-gates branch February 12, 2024 16:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Anything related to C++ code enhancement New feature or request ZX Anything related to the ZX package
Projects
Status: Done
Status: Done
Development

Successfully merging this pull request may close these issues.

🐛 ZX PiExpression arithmetic not working properly ✨ Enhance ZX gate support with newly introduced gates
2 participants