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

[WIP] Partial equivalence checking #487

Closed
wants to merge 68 commits into from

Conversation

reb-ddm
Copy link
Contributor

@reb-ddm reb-ddm commented Nov 27, 2023

Description

Added functionality for partial equivalence checking as described in the paper "Partial Equivalence Checking of Quantum Circuits" by Chen et al. It uses the decision diagram representation of the quantum circuit to check the equivalence conditions as described in the paper.

Some helper functions are in the file Package.hpp and the main partial equivalence checking function is in Verification.hpp.

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 feature New feature or request DD Anything related to the DD package c++ Anything related to C++ code labels Nov 27, 2023
Signed-off-by: burgholzer <burgholzer@me.com>
burgholzer added a commit that referenced this pull request Nov 27, 2023
#488)

## Description

This PR removes a special case handling for when the functionality of a
GRCS circuit is being constructed. As this is a useless and irrelevant
task to conduct in general, there is no real need for special case
treatment.
In addition, this blocks adding optional flags to the
`buildFunctionality` method as desired in #487.

## Checklist:

<!---
This checklist serves as a reminder of a couple of things that ensure
your pull request will be merged swiftly.
-->

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

Signed-off-by: burgholzer <burgholzer@me.com>
src/dd/Verification.cpp Fixed Show fixed Hide fixed
@reb-ddm reb-ddm requested a review from burgholzer February 1, 2024 12:41
Copy link
Member

@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.

Hey 👋🏼

Sorry for the long wait on the review.
This is just a first pass through the changes. I haven't really managed to look through the methods themselves in detail. I'll do that in a follow-up review.
However, this should un-stall your progress and give you some pointers on aspects that might still need some improvement.
Let me know if anything is unclear 👍🏼

include/mqt-core/QuantumComputation.hpp Outdated Show resolved Hide resolved
include/mqt-core/dd/Package.hpp Outdated Show resolved Hide resolved
src/dd/Verification.cpp Outdated Show resolved Hide resolved
src/dd/Verification.cpp Outdated Show resolved Hide resolved
src/dd/Verification.cpp Outdated Show resolved Hide resolved
src/dd/Verification.cpp Outdated Show resolved Hide resolved
test/algorithms/test_partial_equivalence.cpp Outdated Show resolved Hide resolved
test/algorithms/test_partial_equivalence.cpp Outdated Show resolved Hide resolved
test/algorithms/test_partial_equivalence.cpp Outdated Show resolved Hide resolved
test/algorithms/test_partial_equivalence.cpp Outdated Show resolved Hide resolved
@burgholzer
Copy link
Member

Ah, and btw., that remaining clang-tidy warning is a frequent false positive that should hopefully be resolved by now if you rebase your changes on the main branch or merge the main branch in.

@reb-ddm
Copy link
Contributor Author

reb-ddm commented Feb 16, 2024

I noticed that I needed to use static_cast very often in my code, mainly because dd::Qubit is defined as uint16_t and it often needs to be compared to variables of type size_t. But then I found out that there is a type qc::Qubit, which is defined as uint32_t. Changing my code to utilize qc::Qubit instead of dd::Qubit made me remove most of the casts. Is there a reason that these two types with the same name are different? And which one is supposed to be used when?

@burgholzer
Copy link
Member

I noticed that I needed to use static_cast very often in my code, mainly because dd::Qubit is defined as uint16_t and it often needs to be compared to variables of type size_t. But then I found out that there is a type qc::Qubit, which is defined as uint32_t. Changing my code to utilize qc::Qubit instead of dd::Qubit made me remove most of the casts. Is there a reason that these two types with the same name are different?

Yeah and it's kind of implied by the different namespace. In the general qc:: namespace, we want to be able to represent rather large circuits and space is not really too much of a concern. Thus, we use unsigned 32bit integers there, which allows for up to roughly 4 billion qubits.
In the dd:: namespace we want to be as efficient as possible since a variable of that Qubit type is part of every single DD Node. In the past, we used to define that qubit type as int8_t, which would actually limit us to 128 qubits. Since there was some demand for realizing larger simulations than that, we decided to with uint16_t for the DD Qubit type.

And which one is supposed to be used when?

Any code that is part of mqt-core and does not concern DDs should be using the wider qc::Qubit type. Everything that involves DDs can/should be using the narrower dd::Qubit type. However, that's not really dogmatic.

Copy link

codecov bot commented Feb 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.1%. Comparing base (8c199aa) to head (72e1e7c).
Report is 3 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##            main    #487   +/-   ##
=====================================
  Coverage   91.1%   91.1%           
=====================================
  Files        132     132           
  Lines      13794   13809   +15     
  Branches    2166    2176   +10     
=====================================
+ Hits       12575   12589   +14     
- Misses      1219    1220    +1     
Flag Coverage Δ *Carryforward flag
cpp 90.8% <100.0%> (+<0.1%) ⬆️ Carriedforward from 38f4b46
python 99.5% <ø> (ø) Carriedforward from 38f4b46

*This pull request uses carry forward flags. Click here to find out more.

Files Coverage Δ
include/mqt-core/QuantumComputation.hpp 91.8% <100.0%> (+0.1%) ⬆️
include/mqt-core/dd/FunctionalityConstruction.hpp 100.0% <ø> (ø)
src/QuantumComputation.cpp 81.9% <100.0%> (+0.1%) ⬆️
src/dd/FunctionalityConstruction.cpp 96.8% <100.0%> (+<0.1%) ⬆️

... and 2 files with indirect coverage changes

@burgholzer
Copy link
Member

Closing this in favor of #563 for now. We can always re-open if necessary.

@burgholzer burgholzer closed this Mar 18, 2024
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 DD Anything related to the DD package feature New feature or request
Projects
Status: Done
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants