search

cda-tum / mqt-core

MQT Core - The Backbone of the Munich Quantum Toolkit
http://mqt.readthedocs.io/projects/core
MIT License
52 stars 28 forks source link

Generalize concept of removing OpTypes #624

Closed lsschmid closed 3 months ago

lsschmid commented 3 months ago

Description

To not only remove Identities but also other Operations I wrote a function that generalizes that concept and allows one to remove arbitrary operation types.

Checklist:

lsschmid commented 3 months ago

For some reason some other tests fail, not sure why.

Also, probably I don't need the functionality any more, so feel free to delete the PR

burgholzer commented 3 months ago

For some reason some other tests fail, not sure why.

Also, probably I don't need the functionality any more, so feel free to delete the PR

Just from a Quick Look: The main reason for the tests failing is that the previous identity removal would also remove (multi-)controlled identities, while your version only removes single-qubit identities without controls. That should be rather easy to fix though by adding a special case for the identity.

In general, I'd be in favour of getting this merged. It seems like a useful generalisation.

codecov[bot] commented 3 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 91.6%. Comparing base (2782f37) to head (992da1a). Report is 98 commits behind head on main.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/cda-tum/mqt-core/pull/624/graphs/tree.svg?width=650&height=150&src=pr&token=MqstsRKdqp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cda-tum)](https://app.codecov.io/gh/cda-tum/mqt-core/pull/624?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cda-tum) ```diff @@ Coverage Diff @@ ## main #624 +/- ## ===================================== Coverage 91.6% 91.6% ===================================== Files 148 148 Lines 14731 14736 +5 Branches 2365 2365 ===================================== + Hits 13499 13504 +5 Misses 1232 1232 ``` | [Flag](https://app.codecov.io/gh/cda-tum/mqt-core/pull/624/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cda-tum) | Coverage Δ | | |---|---|---| | [cpp](https://app.codecov.io/gh/cda-tum/mqt-core/pull/624/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cda-tum) | `91.3% <100.0%> (+<0.1%)` | :arrow_up: | | [python](https://app.codecov.io/gh/cda-tum/mqt-core/pull/624/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cda-tum) | `99.7% <ø> (ø)` | | | [Files with missing lines](https://app.codecov.io/gh/cda-tum/mqt-core/pull/624?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cda-tum) | Coverage Δ | | |---|---|---| | [src/CircuitOptimizer.cpp](https://app.codecov.io/gh/cda-tum/mqt-core/pull/624?src=pr&el=tree&filepath=src%2FCircuitOptimizer.cpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cda-tum#diff-c3JjL0NpcmN1aXRPcHRpbWl6ZXIuY3Bw) | `88.9% <100.0%> (+<0.1%)` | :arrow_up: | ... and [1 file with indirect coverage changes](https://app.codecov.io/gh/cda-tum/mqt-core/pull/624/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cda-tum)
lsschmid commented 3 months ago

Now it should work. As said, I don't really need it anymore though.

Also: I wrote a comment on the compound operations. Please remove it if it looks fine.