Integrating BQSKit into MQTBench benchmark generation module #286

jagandecapri · 2024-01-22T21:28:20Z

This PR addresses #282. In summary, a new module called bsqkit_helper is created to integrate BSQKit into MQTBench. There are some open issues. Namely, QASM generation of the benchmark is not working at the moment and the test cases currently doesn’t test for QASM generation. There are also other bugs as highlighted below. Numpy was also used in the module which is open for discussion as I saw Numpy is not a dependency in MQTBench currently. Test coverage might be lower as I didn’t add new test cases for the compiler cache and custom gates (ECR and XXPlusYY). Transpilation to only QASM 2.0 gates using Qiskit is needed before converting to bqskit circuit as not all Qiskit circuit gates that is used in the benchmark has an equivalent in bqskit, e.g: mcx gate. Without the transpilation, bsqkit converts the unsupported gate to arbitrarily large block of custom gate which then fails to compile. Runtime for test cases seems to have increased quite considerably with the introduction of bqskit. More details as below:

Bugs in bqskit

QASM string representation

Issue Is it possible to get the QASM string representation of a BQSKit circuit? BQSKit/bqskit#214
QASM string conversion from a bqskit circuit is not working well. It breaks when converting certain control gates
Currently, test cases for bqskit is not testing the QASM string generation

`cry` gate bug

Issue Compilation seems to be stuck with multiple cry gates BQSKit/bqskit#217. Due to this, native and mapped level test cases which has cry gate(s) execute as following:
- pricingcall test case executes indefinitely
- pricingput and random test case sometimes works and sometimes face the same problem of indefinite execution as pricingcall. For random, since it is a random circuit from Qiskit, I suspect when there are multiple cry gates, the test cases starts to go into indefinite execution

New improvements

Custom gates

Implemented two custom gates, ECR and XXPlusYY, following Qiskit little-endian convention. The former is supported natively by OQC Lucy and the latter by Rigetti.
- ECR Gate
  - Type check is disabled for inherited bqskit classes as mypy throws error otherwise. It checks for type information from the inherited classes which doesn’t support type checks I think. Not sure whether this is the right fix.
  - Need verification whether unitary implemented is correct. Obtained it from Qiskit documentation for ECR Gate
  - Created issue in bqskit to add this gate in their library Proposal to include echoed-resonance (ECR) gate into BQSkit. BQSKit/bqskit#216
- XXPlusYY Gate
  - Largely inspired by bqskit FSIM Gate
  - Type check is disabled for inherited bqskit classes as mypy throws error otherwise. It checks for type information from the inherited classes which doesn’t support type checks I think. Not sure whether this is the right fix.
  - There are two types of optimization algorithm that is used by bqskit for parameterized gates. They are DifferentiableUnitary and LocallyOptimizableUnitary. A DifferentiableUnitary can be used in instantiation calls that use a gradient based optimization subroutine. On the other hand,LocallyOptimizableUnitarycan be used in instantiation calls that use a tensor-network based approach like QFactor. More info at here
  - The gate implements only the function for differentiable unitary optimization. From my understanding reading the FSIM and U3 gate implementation, differentiable unitary returns a numpy ndarray containing the partial differentation of the gate’s unitary matrix with respect to each parameter which is what I have implemented. On the other hand, for locally optimizable unitary, I’m not really sure what is the right implementation.
  - Need verification whether unitary in get_unitary and get_grad is correct. Obtained unitary from Qiskit documentation for XXPlusYY gate. As for the gradient, I derived manually and verified with WolframAlpha.
  - This gate uses numpy library in the get_grad function. However, numpy is not listed as dependency in pyproject.toml. I think it gets installed with other library. Should we make it a dependency in MQTBench? if we agree on using numpy, then, some functionality that is using cmath and math module in bsqkit_helper can be replaced with numpy
  - Issue could be created in bsqkit if they are interested to add this gate to their library

CPhase gate

Added CPhase gate for Rigetti native gates

Cache for compiler

Each call to the compile function creates resources to parallelize the compilation across the processors in the system. Hence, there is a waiting time for the setup to occur. Bqskit documentation suggested caching the compiler instance for repeated call of the compile function which is implemented in the CacheCompiler class using the singleton design pattern
The compiler instance is registed with the atexit module to close it when the script exits
Type check is suppressed for __new__ method as ruff was throwing error for the return type which seems to be correct from my point of view

Runtime of test case

I timed the pytest command time pytest -k 'test_quantumcircuit_native_and_mapped_levels' for only bqskit, minus the circuits mentioned in the cry bug section above. It takes ~54 minutes to run this one test function in my laptop. The other test functions execution time seems reasonable

Code structure

The bsqkit_helper class seems to be doing a lot of things, managing cache for the compiler, creating custom gates and benchmark generation. Maybe it could be broken to different files. I didn’t want to change the structure of the repository, so I’ve put all of it in the helper class. Open for discussion.

With that, looking forward to your kind review and discussions. 🙂

…to bqskit

…ble return type

In independent level benchmark, circuit is transpiled to QASM 2.0 compatible. This is because there are some gates used in Qiskit such as `mcx` and `mcphase` which are not correctly decomposed and converted by bqskit.

Error threshold is used for compilation verification. As stated in bqskit tutorial, verification is not for free. It is exponentially costly in time. By default, bqskit doesn't do verification. For MQTBench, verification may not be necessary.

src/mqt/bench/bqskit_helper.py

+    target_directory: str = "./",
+    target_filename: str = "",
+) -> Circuit:
+    ...


src/mqt/bench/bqskit_helper.py

+    target_directory: str = "./",
+    target_filename: str = "",
+) -> bool:
+    ...


src/mqt/bench/bqskit_helper.py

+    target_directory: str = "./",
+    target_filename: str = "",
+) -> Circuit:
+    ...


src/mqt/bench/bqskit_helper.py

+    target_directory: str = "./",
+    target_filename: str = "",
+) -> bool:
+    ...


src/mqt/bench/bqskit_helper.py

+    target_directory: str = "./",
+    target_filename: str = "",
+) -> Circuit:
+    ...


src/mqt/bench/bqskit_helper.py

+    target_directory: str = "./",
+    target_filename: str = "",
+) -> bool:
+    ...


Bqskit code in benchmark generator was not properly setup. Test cases were also not properly setup. This commit fixes both. Note that the test convert bqskit circuit to qiskit is commented out because of the bug in bqskit BQSKit/bqskit#214

The test case fix is just to get it passing. It doesn't test the actual QASM file generation in the functions. I have left TODOs in the code to remind me to add these tests later.

TODOs are added to test_bench to indicate the test case code that needs to be updated once the bugs in bqskit library are fixed.

jagandecapri · 2024-01-23T11:54:10Z

Fixed failing test cases due to bugs in code. Some test cases were left to failed intentionally to indicate the part that needs to be updated when the bugs in bqskit library are fixed. Alternatively, in some cases, I have left TODOs comments in test_bench.py to indicate the parts that needs updating once the bugs in bqskit library are fixed.

nquetschlich

Thanks a lot for your effort and this PR, @jagandecapri. I am very happy that we will (soon) also support BQSKit as another compiler. I have just mentioned a few things that caught my attention. Additionally I was wondering, whether we really need all the newly introduced type ignores. Please carefully double check, whether those are actually necessary.

nquetschlich · 2024-01-24T07:53:43Z

pyproject.toml

@@ -19,6 +19,7 @@ requires-python = ">=3.9"
 dynamic = ["version"]

 dependencies = [
+    "bqskit>=1.1.1",


It would be great to add an upper cap as well.

nquetschlich · 2024-01-24T07:55:28Z

src/mqt/bench/benchmark_generator.py

@@ -141,6 +148,31 @@ def generate_mapped_levels(
    ) -> None:
        for provider in get_available_providers():
            for device in provider.get_available_devices():
+                for opt_level in [1, 2, 3, 4]:


Should be iterate over BQSKitSettings that cover those optimization levels? I am aware that this is not the case for Qiskit and TKET as well but desired (see #88). Therefore, I would like this new implementation to already follow this direction.

nquetschlich · 2024-01-24T07:57:25Z

src/mqt/bench/benchmark_generator.py

@@ -198,6 +230,28 @@ def generate_native_gates_levels(
        parameter_space: list[tuple[int, str]] | list[int] | list[str] | range,
    ) -> None:
        for provider in get_available_providers():
+            for opt_level in [1, 2, 3, 4]:


nquetschlich · 2024-01-24T07:59:54Z

src/mqt/bench/bqskit_helper.py

+    from mqt.bench.devices import Device, Provider
+
+
+class CachedCompiler:


Why is this needed? Do we really need to have this definition in our code and cannot use something from BQSKit?

nquetschlich · 2024-01-24T08:09:51Z

src/mqt/bench/bqskit_helper.py

+        "cx": CNOTGate(),
+        "cz": CZGate(),
+        "ecr": ECRGate(),
+        "xx_plus_yy": XXPlusYYGate(),


Could you also make sure that this gate is implemented for TKET as well in the tket_helper.py file? Same for the cp gate.

nquetschlich · 2024-01-24T08:12:36Z

src/mqt/bench/bqskit_helper.py

+    except Exception as e:
+        print("BQSKit Exception Indep: ", e)
+        return False
+


Where is the BQSKit compilation happening? I don't see it here.

nquetschlich · 2024-01-24T08:16:05Z

src/mqt/bench/bqskit_helper.py

+        print("BQSKit Exception NativeGates: ", e)
+        return False
+
+    gate_set = provider.get_native_gates()


all those compilation commands should be within the try block

nquetschlich · 2024-01-24T08:17:27Z

tests/test_bench.py

@@ -70,15 +73,18 @@ def sample_filenames() -> list[str]:
        "ae_indep_qiskit_10.qasm",
        "ghz_nativegates_rigetti_qiskit_opt3_54.qasm",
        "ae_indep_tket_93.qasm",
+        "ae_indep_bqskit_93.qasm",


Maybe a smaller circuit would be better suited for BQSKit because of their runtime (especially for optimization levels >2)

nquetschlich · 2024-01-24T08:19:01Z

tests/test_bench.py

-        ("indep", 2),
-        ("mapped", 9),
+        ("indep", 3),
+        ("mapped", 11),


Why have the test case sizes changed?

jagandecapri · 2024-01-31T11:27:39Z

To add bqskit version to benchmark files https://github.com/jagandecapri/MQTBench/blob/918a3c0f2eea84ab935d896513b322849a81c3e7/src/mqt/bench/utils.py#L197-L200

jagandecapri added 30 commits January 16, 2024 16:56

➕ Add bqskit dependency

66c31de

🎉 Commit BQSKit helper module

0bde489

✅ Add bqskit tests to native and mapped levels

60839c5

🔧 Add bqskit to mypy overrides in pyproject.toml

3b20140

✅ Add bqskit tests to get benchmark test

ef31b7b

✅ Add bqskit test for save qasm in alternative location

773a2f6

✅ Add bqskit test case for count occurrences from sample file

9dd359e

✅ Add bqskit test case to count qubit number

c494956

✅ Add bqskit test to count qubit number for mapped circuit

9b281e9

✨ Commit draft of bqskit helper module

73bbb83

✨ Commit draft of bqskit integration into benchmark generator

9b627a1

✅ Rearrange test codes

eeb55c0

🐛 Qiskit sub-routines, e.g: QFT, need to be decomposed before giving …

db47eda

…to bqskit

🔥 Remove commented out code in bqskit_helper.py

09a0b76

📝 Update benchmark_generator.py to include BQSKit::Circuit as a possi…

a848561

…ble return type

📝 Fix compiler order in benchmark_generator.py

b973e61

🐛 Transpile circuit to QASM 2.0 compatible

4dbe280

In independent level benchmark, circuit is transpiled to QASM 2.0 compatible. This is because there are some gates used in Qiskit such as `mcx` and `mcphase` which are not correctly decomposed and converted by bqskit.

🔥 Remove unnecessary code in indep level

f6941e3

✨ Add custom ECRGate to bqskit_helper.py

b4d46f6

✨ Add ECRGate to gate set

f565786

🐛 Transpile circuit to QASM 2.0 compatible

4b58409

🐛 Transpile circuit to QASM 2.0 compatible

4471f03

🐛 Add seed parameter to compile function for reproducibility

5631883

⚡️ Add CachedCompiler class to improve performance

8251496

⚡️ Refactor ECRGate unitary matrix initialization

2175e16

✨ Add XXPlusYY gate for Rigetti backend

c4d6d51

✨ Add CPGate to gate set for Rigetti

086d9d2

✨ Update benchmark generator to include bqskit_helper in indep level

56698e9

✅ Clean-up test cases for indep level

73058ee

✅ Update native and mapped level test case

34b98ba

github-advanced-security bot found potential problems Jan 22, 2024

View reviewed changes

jagandecapri added 4 commits January 23, 2024 11:03

✅ Fix failing test case for saving qasm to alt loc

f5b0776

The test case fix is just to get it passing. It doesn't test the actual QASM file generation in the functions. I have left TODOs in the code to remind me to add these tests later.

✅ Add TODOs to test_bench.py

f426caf

TODOs are added to test_bench to indicate the test case code that needs to be updated once the bugs in bqskit library are fixed.

Merge branch 'main' into bqskit_benchmark_generator

918a3c0

jagandecapri mentioned this pull request Jan 23, 2024

Integrating BQSKit into MQTBench bench viewer (website) module #288

Draft

jagandecapri marked this pull request as draft January 23, 2024 16:15

nquetschlich requested changes Jan 24, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Integrating BQSKit into MQTBench benchmark generation module #286

Integrating BQSKit into MQTBench benchmark generation module #286

jagandecapri commented Jan 22, 2024

jagandecapri commented Jan 23, 2024

nquetschlich left a comment

nquetschlich Jan 24, 2024

nquetschlich Jan 24, 2024

nquetschlich Jan 24, 2024

nquetschlich Jan 24, 2024

nquetschlich Jan 24, 2024

nquetschlich Jan 24, 2024

nquetschlich Jan 24, 2024

nquetschlich Jan 24, 2024

nquetschlich Jan 24, 2024

jagandecapri commented Jan 31, 2024

		from mqt.bench.devices import Device, Provider


		class CachedCompiler:

Integrating BQSKit into MQTBench benchmark generation module #286

Are you sure you want to change the base?

Integrating BQSKit into MQTBench benchmark generation module #286

Conversation

jagandecapri commented Jan 22, 2024

Bugs in bqskit

QASM string representation

cry gate bug

New improvements

Custom gates

CPhase gate

Cache for compiler

Runtime of test case

Code structure

jagandecapri commented Jan 23, 2024

nquetschlich 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

jagandecapri commented Jan 31, 2024

`cry` gate bug