✨ Collect clifford blocks #885

jannikpflieger · 2025-03-24T07:52:50Z

Description

Added the ability to separate the collection of Clifford and non Clifford Blocks by adding a new function parameter. For that a isCliffordCheck was added as well. The isClifford check is very rough but includes all common Clifford gates. However, it does not include rotation gates that with the right angle could be considered Clifford. It does not include custom gates that could be Clifford as well.

The separation is done by finalizing a block when we iterate over a non-Clifford Block. When we want to collect Clifford and non Clifford gates separately then the function handles non-Clifford gates as "cannot be processed" and the canProcess variable is set to false. This ensures that we still have large CliffordBlocks but we have only single non CliffordBlocks. Essentially separating the non-CliffordBlocks from all Clifford and other non-CliffordBlocks.

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.

Signed-off-by: Jannik Pflieger <[email protected]>

codecov · 2025-03-24T07:57:39Z

Codecov Report

Attention: Patch coverage is 64.70588% with 6 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/ir/operations/CompoundOperation.cpp	0.0%	3 Missing ⚠️
src/ir/operations/StandardOperation.cpp	83.3%	2 Missing ⚠️
include/mqt-core/ir/operations/Operation.hpp	0.0%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

test/circuit_optimizer/test_collect_blocks.cpp

burgholzer

Thanks for this initial PR 🙏🏼
I just went through everything and left a couple of inline comments. This still needs a bit of work before it is ready to be merged.
Don't hesitate to ask if you should have any questions.

burgholzer · 2025-04-07T19:24:08Z

include/mqt-core/circuit_optimizer/CircuitOptimizer.hpp

@@ -90,7 +90,8 @@ class CircuitOptimizer {
   * @param qc the quantum circuit
   * @param maxBlockSize the maximum size of a block
   */
-  static void collectBlocks(QuantumComputation& qc, std::size_t maxBlockSize);
+  static void collectBlocks(QuantumComputation& qc, std::size_t maxBlockSize,
+                            bool collectCliffords);


Suggested change

bool collectCliffords);

bool onlyCollectCliffords = false);

Maybe that name would be a little more suitable?
Also, this parameter is missing a description in the corresponding docstring.
Finally, this should default to false, otherwise this would be a breaking change.

burgholzer · 2025-04-07T19:31:37Z

include/mqt-core/ir/operations/Operation.hpp

+  [[nodiscard]] virtual bool isClifford() const {
+    const OpType opType = this->getType();
+    const bool controlled = this->isControlled();
+    const std::size_t numberOfControls = this->getNcontrols();
+    if ((controlled) && ((numberOfControls >= 2))) {
+      return false;
+    }
+    switch (opType) {
+    case I:
+    case X:
+    case Y:
+    case Z:
+    case H:
+    case S:
+    case Sdg:
+    case SX:
+    case SXdg:
+    case DCX:
+    case SWAP:
+    case iSWAP:
+    case ECR:
+      return true;
+    default:
+      return false;
+    }
+  }


This is not yet correct. As it stands, this would, e.g., classify a controlled-S gate as Clifford.
While the aspiration of this function is not to yield absolutely no false-negatives, it must not yield false positives.

you also do not need the this-> in the member function.

the function here should actually simply return false and an overload of the method should be implemented in the StandardOperation class that contains this implementation. The CompoundOperation class will also need an overload that checks if every operation in the compound operation is Clifford. Other similar functions are already implemented in the respective classes and serve as a blueprint.

while we are at it, would you mind adding a docstring to any method that you add to the code base?

you can tweak the code slightly to improve the checks. Identity gates as well as barriers are supported with arbitrary numbers of qubits. thus, you should check for these first. Then maybe check for the gates that allow a single control (X, Y, Z) and then for the gates that must have no controls (the rest from the list above).

burgholzer · 2025-04-07T19:33:25Z

src/circuit_optimizer/CircuitOptimizer.cpp

+    bool isClifford = false;

    // check if the operation can be processed
    if (!op->isUnitary()) {
      canProcess = false;
    }

+    // check if the operation is a Clifford operation
+    if (collectCliffords && op->isClifford()) {
+      isClifford = true;
+    }
+
+    if (collectCliffords && !isClifford) {
+      canProcess = false;
+    }


Suggested change

bool isClifford = false;

// check if the operation can be processed

if (!op->isUnitary()) {

canProcess = false;

}

// check if the operation is a Clifford operation

if (collectCliffords && op->isClifford()) {

isClifford = true;

}

if (collectCliffords && !isClifford) {

canProcess = false;

}

// check if the operation can be processed

if (!op->isUnitary() || (collectCliffords && !op->isClifford()) {

canProcess = false;

}

much simpler

burgholzer · 2025-04-07T19:35:28Z

test/circuit_optimizer/test_collect_blocks.cpp

All the changes in this file where

qc::CircuitOptimizer::collectBlocks(..., ..., false);

Should be reverted to remain unchanged by this PR based on the default argument for the pass.

burgholzer · 2025-04-07T19:35:55Z

test/circuit_optimizer/test_collect_blocks.cpp

+TEST(CollectBlocks, emptyCliffordCircuit) {
+  QuantumComputation qc(1);
+  qc::CircuitOptimizer::collectBlocks(qc, 1, true);


This is a pretty useless test. The new flag has no influence at all on empty circuits.

burgholzer · 2025-04-07T19:36:46Z

test/circuit_optimizer/test_collect_blocks.cpp

+TEST(CollectBlocks, singleCliffordGate) {
+  QuantumComputation qc(1);
+  qc.t(0);
+  std::cout << qc << "\n";
+  qc::CircuitOptimizer::collectBlocks(qc, 1, true);


The T gate is not a Clifford gate. This seems to be a non-sense test. Please adjust to make this a reasonable test.

burgholzer · 2025-04-07T19:38:03Z

test/circuit_optimizer/test_collect_blocks.cpp

+TEST(CollectBlocks, collectMultipleSingleQubitCliffordGates) {
+  QuantumComputation qc(2);
+  qc.h(0);
+  qc.h(1);
+  qc.t(0);
+  qc.t(1);
+  qc.x(0);
+  qc.x(1);
+  std::cout << qc << "\n";
+  qc::CircuitOptimizer::collectBlocks(qc, 1, true);
+  std::cout << qc << "\n";
+  EXPECT_EQ(qc.size(), 6);
+}


The test name implies that this tests some kind of collection. However, the actual test is set up to not change a single thing. Please revise the test so that it is actually useful.

burgholzer · 2025-04-07T19:41:13Z

test/circuit_optimizer/test_collect_blocks.cpp

+TEST(CollectBlocks, collectMultipleCliffordGates) {
+  QuantumComputation qc(2);
+  qc.h(0);
+  qc.h(1);
+  qc.t(0);
+  qc.x(0);
+  qc.x(1);
+  std::cout << qc << "\n";
+  qc::CircuitOptimizer::collectBlocks(qc, 2, true);
+  std::cout << qc << "\n";
+  EXPECT_EQ(qc.size(), 4);
+  EXPECT_TRUE(qc.front()->isCompoundOperation());
+}


This test does not seem quite right.
Looking at the circuit, I would have imagined there to be 3 blocks in the resulting circuit (H(0), H(1), X(1)), (T(0)), and (X(0)).
Please check the implementation again. and devise reasonable tests.
Instead of simply asserting the number of operations, assert what the actual operations are to make sure that the pass is doing what it is supposed to do.

burgholzer · 2025-04-07T19:42:52Z

test/circuit_optimizer/test_collect_blocks.cpp

+TEST(CollectBlocks, collectTwoQubitCliffordGates) {
+  QuantumComputation qc(2);
+  qc.h(0);
+  qc.h(1);
+  qc.cx(0, 1);
+  qc.t(0);
+  qc.x(0);
+  qc.x(1);
+  std::cout << qc << "\n";
+  qc::CircuitOptimizer::collectBlocks(qc, 2, true);
+  std::cout << qc << "\n";
+  EXPECT_EQ(qc.size(), 4);
+  EXPECT_TRUE(qc.front()->isCompoundOperation());
+}


A couple more tests are in order. Tests should cover at least every Clifford gate once and every non-Clifford gate once.

burgholzer · 2025-04-07T19:44:04Z

test/ir/test_operation.cpp

+TEST(Operation, IsClifford) {
+  const qc::StandardOperation x(0, qc::X);
+  EXPECT_TRUE(x.isClifford());
+  const qc::StandardOperation cx(0, 1, qc::X);
+  EXPECT_TRUE(cx.isClifford());
+  const qc::StandardOperation y(0, 1, qc::Y);
+  EXPECT_TRUE(y.isClifford());
+  const qc::StandardOperation z(0, 1, qc::Z);
+  EXPECT_TRUE(z.isClifford());
+  const qc::StandardOperation t(0, qc::T);
+  EXPECT_FALSE(t.isClifford());
+  const qc::Controls controls = {qc::Control(0), qc::Control(1)};
+  const qc::StandardOperation ccx(controls, 2, qc::X);
+  EXPECT_FALSE(ccx.isClifford());
+  const qc::StandardOperation ccy(controls, 2, qc::Y);
+  EXPECT_FALSE(ccy.isClifford());
+  const qc::StandardOperation ccz(controls, 2, qc::Z);
+  EXPECT_FALSE(ccz.isClifford());
+  const qc::StandardOperation h(0, qc::H);
+  EXPECT_TRUE(h.isClifford());
+  const qc::StandardOperation s(0, qc::S);
+  EXPECT_TRUE(s.isClifford());
+  const qc::StandardOperation sdg(0, qc::Sdg);
+  EXPECT_TRUE(sdg.isClifford());
+  const qc::StandardOperation sx(0, qc::SX);
+  EXPECT_TRUE(sx.isClifford());
+  const qc::StandardOperation sxdg(0, qc::SXdg);
+  EXPECT_TRUE(sxdg.isClifford());
+  const qc::StandardOperation dcx(0, 1, qc::DCX);
+  EXPECT_TRUE(dcx.isClifford());
+  const qc::StandardOperation swap(0, 1, qc::SWAP);
+  EXPECT_TRUE(swap.isClifford());
+  const qc::StandardOperation iswap(0, 1, qc::iSWAP);
+  EXPECT_TRUE(iswap.isClifford());
+  const qc::StandardOperation ecr(0, 1, qc::ECR);
+  EXPECT_TRUE(ecr.isClifford());
+}


These are not really conclusive (see comment where the function is defined).

burgholzer · 2025-04-28T18:46:03Z

@jannikpflieger any updates on this? it's been three weeks since the review above 😌

jannikpflieger · 2025-04-28T20:18:49Z

@burgholzer still doing it, since I was on vacation + exam, but I'm on it! I'm back since yesterday evening :D

…-core-cliffordblockfinder into collectCliffordBlocks

…llect blocks, changed isClifford function, added docstring

…-core-cliffordblockfinder into collectCliffordBlocks

jannikpflieger and others added 8 commits March 11, 2025 08:24

isCliffordCheck

212533c

added isCliffordOperation

3f64b2f

edited collectBlocks to handle CliffordBlocks

5db7ac5

Added Tests and redefined if statements in opertaon

4995e47

fixed Testcases

62004e5

Update ci.yml

1656046

Signed-off-by: Jannik Pflieger <[email protected]>

Update ci.yaml again

8c811bd

Merge branch 'cda-tum:main' into collectCliffordBlocks

5812c72

github-project-automation bot added this to MQT Core Mar 24, 2025

github-project-automation bot moved this to In Progress in MQT Core Mar 24, 2025

🎨 pre-commit fixes

d9d2132

github-advanced-security bot found potential problems Mar 24, 2025

View reviewed changes

test/circuit_optimizer/test_collect_blocks.cpp Fixed Show fixed Hide fixed

jannikpflieger and others added 4 commits March 24, 2025 16:46

more Test Coverage and fix linting errors

3282c67

Merge branch 'cda-tum:main' into collectCliffordBlocks

43d18ae

removed outcommented code

088ada5

Added import and fixed variable const

486e225

burgholzer changed the title ~~Collect clifford blocks~~ ✨ Collect clifford blocks Apr 7, 2025

burgholzer added enhancement New feature or request Core Anything related to the Core library and IR c++ Anything related to C++ code labels Apr 7, 2025

burgholzer added this to the MQT Core milestone Apr 7, 2025

burgholzer requested changes Apr 7, 2025

View reviewed changes

jannikpflieger and others added 5 commits April 29, 2025 06:06

Merge branch 'munich-quantum-toolkit:main' into collectCliffordBlocks

66ed0fc

changed parameter name and added docstring

e1ffb25

Merge branch 'collectCliffordBlocks' of github.com:jannikpflieger/mqt…

0ae9525

…-core-cliffordblockfinder into collectCliffordBlocks

🎨 pre-commit fixes

d7264a0

reverted original tests back to normal, changed function header of co…

a12f11a

…llect blocks, changed isClifford function, added docstring

jannikpflieger and others added 11 commits April 29, 2025 06:49

Merge branch 'collectCliffordBlocks' of github.com:jannikpflieger/mqt…

7eb95fc

…-core-cliffordblockfinder into collectCliffordBlocks

🎨 pre-commit fixes

5555b8e

Moved IsCliffordfunction to StandardOperation and CompoundOperation

e5de02d

Merge branch 'collectCliffordBlocks' of github.com:jannikpflieger/mqt…

6e728cb

…-core-cliffordblockfinder into collectCliffordBlocks

🎨 pre-commit fixes

71e8bc6

Corrected Test for isClifford operations

f771766

Merge branch 'collectCliffordBlocks' of github.com:jannikpflieger/mqt…

d97eef5

…-core-cliffordblockfinder into collectCliffordBlocks

🎨 pre-commit fixes

227d2ec

Removed usless test added more meanigful ones

5ce85b4

Merge branch 'collectCliffordBlocks' of github.com:jannikpflieger/mqt…

326c5b9

…-core-cliffordblockfinder into collectCliffordBlocks

🎨 pre-commit fixes

ace9eaf

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨ Collect clifford blocks #885

✨ Collect clifford blocks #885

jannikpflieger commented Mar 24, 2025 •

edited

Loading

codecov bot commented Mar 24, 2025 •

edited

Loading

burgholzer left a comment

burgholzer Apr 7, 2025

burgholzer Apr 7, 2025

burgholzer Apr 7, 2025

burgholzer Apr 7, 2025

burgholzer Apr 7, 2025

burgholzer Apr 7, 2025

burgholzer Apr 7, 2025

burgholzer Apr 7, 2025

burgholzer Apr 7, 2025

burgholzer Apr 7, 2025

burgholzer commented Apr 28, 2025

jannikpflieger commented Apr 28, 2025

✨ Collect clifford blocks #885

Are you sure you want to change the base?

✨ Collect clifford blocks #885

Conversation

jannikpflieger commented Mar 24, 2025 • edited Loading

Description

Checklist:

codecov bot commented Mar 24, 2025 • edited Loading

Codecov Report

burgholzer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

burgholzer commented Apr 28, 2025

jannikpflieger commented Apr 28, 2025

jannikpflieger commented Mar 24, 2025 •

edited

Loading

codecov bot commented Mar 24, 2025 •

edited

Loading